Release 1.6.0

With --raw there is a workaround.

The tolerant approach was cool and nice until you want to edit something
non-interactively and get the warning telling you you might have lost
data after the file was written. Failing fast is most likely the better
option here.

CHANGELOG.md

@@ -1,6 +1,46 @@
 opustags changelog
 ==================
 
+1.6.0 - 2021-01-01
+------------------
+
+- UTF-8 conversion errors are now fatal.
+- Introduce --raw for disabling encoding conversions.
+- Improve platform compatibility.
+
+This also happens to be opustags’s 8-year anniversary!
+
+1.5.1 - 2020-11-21
+------------------
+
+- Improve BSD support.
+
+1.5.0 - 2020-11-08
+------------------
+
+- Introduce --edit for interactive edition.
+
+1.4.0 - 2020-10-04
+------------------
+
+- Preserve permissions when overwriting files.
+- Support multiple files with --in-place.
+- Fix BSD support.
+
+Thanks to Reuben Thomas for contributing the pièce de résistance of this
+release!
+
+1.3.0 - 2019-02-02
+------------------
+
+- Support for non-Unicode systems. Tags are automatically converted to and from the system locale.
+- It is now possible to delete specific NAME=VALUE pairs.
+- Option `--set-all` is now stricter and aborts with an error if the input is not valid.
+- Printing tags will display a warning if the tags contain control characters.
+
+opustags is now more aware of its limitations, and will print more helpful error messages when
+trying to edit an unsupported file. It is also more cautious against corrupted streams.
+
 1.2.0 - 2018-11-25
 ------------------
 

CMakeLists.txt

@@ -2,31 +2,49 @@ cmake_minimum_required(VERSION 3.9)
 
 project(
 	opustags
-	VERSION 1.2.0
+	VERSION 1.6.0
 	LANGUAGES CXX
 )
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
+# opustags is mainly developed with glibc, which introduces a few
+# incompatibilites with BSDs, like getline not being defined by default.
+# _GNU_SOURCE should trigger BSD’s libc GNU compatibility mode to fix that.
+add_definitions(-D_GNU_SOURCE)
+
 find_package(PkgConfig REQUIRED)
 pkg_check_modules(OGG REQUIRED ogg)
 add_compile_options(${OGG_CFLAGS})
+link_directories(${OGG_LIBRARY_DIRS})
+
+include(FindIconv)
+
+# We need endian.h on Linux, and sys/endian.h on BSD.
+include(CheckIncludeFileCXX)
+check_include_file_cxx(endian.h HAVE_ENDIAN_H)
+check_include_file_cxx(sys/endian.h HAVE_SYS_ENDIAN_H)
+
+include(CheckStructHasMember)
+check_struct_has_member("struct stat" st_mtim sys/stat.h HAVE_STAT_ST_MTIM LANGUAGE CXX)
+check_struct_has_member("struct stat" st_mtimespec sys/stat.h HAVE_STAT_ST_MTIMESPEC LANGUAGE CXX)
 
 configure_file(src/config.h.in config.h @ONLY)
-include_directories(BEFORE src "${CMAKE_BINARY_DIR}")
+include_directories(BEFORE src "${CMAKE_BINARY_DIR}" ${OGG_INCLUDE_DIRS} ${Iconv_INCLUDE_DIRS})
 
 add_library(
-	libopustags
-	OBJECT
+	ot
+	STATIC
 	src/cli.cc
 	src/ogg.cc
 	src/opus.cc
+	src/system.cc
 )
-target_link_libraries(libopustags PUBLIC ${OGG_LIBRARIES})
+target_link_libraries(ot PUBLIC ${OGG_LIBRARIES} ${Iconv_LIBRARIES})
 
 add_executable(opustags src/opustags.cc)
-target_link_libraries(opustags libopustags)
+target_link_libraries(opustags ot)
 
 include(GNUInstallDirs)
 install(TARGETS opustags DESTINATION "${CMAKE_INSTALL_BINDIR}")

CONTRIBUTING.md

@@ -1,6 +1,7 @@
 # Contributing to opustags
 
-opustags is slowing getting more mature, and contributions are welcome.
+opustags should now be mature enough, and contributions for new features are
+welcome.
 
 Before you open a pull request, you might want to talk about the change you'd
 like to make to make sure it's relevant. In that case, feel free to open an
@@ -41,19 +42,30 @@ questioned, and it was thus abandoned for a few years. Judging by the
 inquiries and contributions, albeit few, on GitHub, it looks like it remains
 relevant, so let's dust it off a bit.
 
-Today, opustags is written in C++14 and features a unit test suite in C++, and
+Today, opustags is written in C++ and features a unit test suite in C++, and
 an integration test suite in Perl. The code was refactored, organized into
 modules, and reviewed for safety.
 
-The next release will focus on correctness, with the following technical
-objectives:
+1.3.0 was focused on correctness, and detects edge cases as early as possible,
+instead of hoping something will eventually fail if something is weird.
 
-1. Validate the comments: field name in ASCII and value in UTF-8.
-2. Allow selecting the stream to edit, instead of assuming the Ogg contains only
-   one Opus stream.
-3. Provide an --escape option for escaping the newlines inside comment strings.
-4. Take into account the system's encoding: the tags must always be stored as
-   UTF-8, and converted from and to the console encoding when reading input or
-   printing.
-5. Maybe provide a --binary option to dump the raw OpusTags packet, that can be
-   combined to --set-all to read it back.
+## Candidate features
+
+The code contains a few `\todo` markers where something could be improved in the
+code.
+
+More generally, here are a few features that could be added in the future:
+
+- Discouraging non-ASCII field names.
+- Logicial stream listing and selection for multiplexed files.
+- Escaping control characters with --escape.
+- Dump binary packets with --binary.
+- Edition of the vendor string.
+- Edition of the arbitrary binary block past the comments.
+- Support for OpusTags packets spanning multiple pages (> 64 kB).
+- Interactive edition of comments inside the EDITOR (--edit).
+- Support for cover arts.
+- Load tags from a file with --set-all=tags.txt.
+- Colored output.
+
+Don't hesitate to contact me before you do anything, I'll give you directions.

README.md

@@ -1,33 +1,32 @@
 opustags
 ========
 
-View and edit Opus comments.
+View and edit Ogg Opus comments.
 
-The current code quality of this project is getting better, and is suitable for reliably editing any
-Opus file provided it does not contain other multiplexed streams. Only UTF-8 is currently supported.
+opustags is designed to be fast and as conservative as possible, to the point that if you edit tags
+then edit them again to their previous values, you should get a bit-perfect copy of the original
+file. No under-the-cover operation like writing "edited with opustags" or timestamp tagging will
+ever be performed.
 
-Until opustags becomes top-quality software, if it ever does, you might want to
-check out these more mature tag editors:
+It currently has the following limitations:
 
-- [EasyTAG](https://wiki.gnome.org/Apps/EasyTAG)
-- [Beets](http://beets.io/)
-- [Picard](https://picard.musicbrainz.org/)
-- [puddletag](http://docs.puddletag.net/)
-- [Quod Libet](https://quodlibet.readthedocs.io/en/latest/)
-- [Goggles Music Manager](https://gogglesmm.github.io/)
+- The total size of all tags cannot exceed 64 kB, the maximum size of one Ogg page.
+- Multiplexed streams are not supported.
+- Newlines inside tags are not supported by `--set-all`.
 
-See also these libraries if you need a lower-level access:
-
-- [TagLib](http://taglib.org/)
-- [mutagen](https://mutagen.readthedocs.io/en/latest/)
+If you'd like one of these limitations lifted, please do open an issue explaining your use case.
+Feel free to ask for new features too.
 
 Requirements
 ------------
 
-* a C++14 compiler,
-* CMake,
 * a POSIX-compliant system,
-* libogg.
+* a C++17 compiler,
+* CMake ≥ 3.9,
+* libogg 1.3.3.
+
+The version numbers are indicative, and it's very likely opustags will build and work fine with
+other versions too, as CMake and libogg are quite mature.
 
 Installing
 ----------
@@ -49,17 +48,20 @@ Documentation
 
     Usage: opustags --help
            opustags [OPTIONS] FILE
+           opustags OPTIONS -i FILE...
            opustags OPTIONS FILE -o FILE
 
     Options:
-      -h, --help              print this help
-      -o, --output FILE       set the output file
-      -i, --in-place          overwrite the input file instead of writing a different output file
-      -y, --overwrite         overwrite the output file if it already exists
-      -a, --add FIELD=VALUE   add a comment
-      -d, --delete FIELD      delete all previously existing comments of a specific type
-      -D, --delete-all        delete all the previously existing comments
-      -s, --set FIELD=VALUE   replace a comment (shorthand for --delete FIELD --add FIELD=VALUE)
-      -S, --set-all           replace all the comments with the ones read from standard input
+      -h, --help                    print this help
+      -o, --output FILE             specify the output file
+      -i, --in-place                overwrite the input files
+      -y, --overwrite               overwrite the output file if it already exists
+      -a, --add FIELD=VALUE         add a comment
+      -d, --delete FIELD[=VALUE]    delete previously existing comments
+      -D, --delete-all              delete all the previously existing comments
+      -s, --set FIELD=VALUE         replace a comment
+      -S, --set-all                 import comments from standard input
+      -e, --edit                    edit tags interactively in VISUAL/EDITOR
+      --raw                         disable encoding conversion
 
 See the man page, `opustags.1`, for extensive documentation.

opustags.1

@@ -1,6 +1,6 @@
-.TH opustags 1 "November 2018" "@PROJECT_NAME@ @PROJECT_VERSION@"
+.TH opustags 1 "December 2018" "@PROJECT_NAME@ @PROJECT_VERSION@"
 .SH NAME
-opustags \- Opus comment editor
+opustags \- Ogg Opus tag editor
 .SH SYNOPSIS
 .B opustags --help
 .br
@@ -10,12 +10,17 @@ opustags \- Opus comment editor
 .br
 .B opustags
 .I OPTIONS
+.B -i
+.R \fIFILE\fP...
+.br
+.B opustags
+.I OPTIONS
 .B -o
 .I OUTPUT INPUT
 .SH DESCRIPTION
 .PP
-\fBopustags\fP can read and edit the comment header of an Opus file.
-It basically has two modes: read-only, and read-write for tag edition.
+\fBopustags\fP can read and edit the comment header of an Ogg Opus file.
+It basically has two modes: read-only, and read-write for tag editing.
 .PP
 In read-only mode, only the beginning of \fIINPUT\fP is read, and the tags are
 printed on standard output.
@@ -23,10 +28,12 @@ printed on standard output.
 You can use the options below to edit the tags before printing them.
 This could be useful to preview some changes before writing them.
 .PP
-In edition mode, you need to specify an output file (or \fB-\fP for standard output). It must be
-different from the input file. To overwrite the input file, use \fB--in-place\fP.
+In editing mode, you need to specify an output file with \fB--output\fP, or use \fB--in-place\fP to
+overwrite the input files. If the output is a regular file, the result is first written to a
+temporary file and then moved to its final location on success. On error, the temporary output file
+is deleted.
 .PP
-Tag edition can be performed with the \fB--add\fP, \fB--delete\fP and \fB--set\fP
+Tag editing can be performed with the \fB--add\fP, \fB--delete\fP and \fB--set\fP
 options. Options can be specified in any order and don’t conflict with each other.
 First the specified tags are deleted, then the new tags are added.
 .PP
@@ -38,9 +45,12 @@ If you want to replace all the tags, you can use the \fB--set-all\fP option whic
 The format is the same as the one used for output: newline-separated \fIFIELD=Value\fP assignment.
 All the previously existing tags as deleted.
 .PP
-\fBWarning:\fP the Opus format specifications requires tags to be encoded in
-\fBUTF-8\fP. This tool ignores the system locale, assuming the encoding is
-set to UTF-8, and assume that tags are already encoded in UTF-8.
+The Opus format specifications requires that tags are encoded in UTF-8, so that's the only encoding
+opustags supports. If your system encoding is different, the tags are automatically converted to and
+from your system locale. When the conversion is lossy, the incompatible characters are
+transliterated and a warning is displayed. Even if you edit an Opus file whose tags contains
+characters unsupported by your system encoding, the original UTF-8 values will be preserved for the
+tags you don't explictly modify.
 .SH OPTIONS
 .TP
 .B \-h, \-\-help
@@ -52,20 +62,20 @@ The input file will be read, its tags edited, then written to the specified outp
 \fIFILE\fP is \fB-\fP then the resulting Opus file will be written to standard output.
 The output file can’t be the same as the input file.
 .TP
-.B \-i, \-\-in-place\fR[=\fP\fISUFFIX\fP\fR]\fP
-Use this when you want to modify the input file in-place. opustags will create a temporary output
-file with the specified suffix (.otmp by default), and move it to the location of the input file on
-success. If a file with the same name as the temporary file already exists, it will be overwritten
-without warning.
+.B \-i, \-\-in-place
+Overwrite the input file instead of creating a separate output file. It has the same effect as
+setting \fB--output\fP to the same path as the input file and enabling \fB--overwrite\fP.
 This option conflicts with \fB--output\fP.
 .TP
 .B \-y, \-\-overwrite
-By default, \fBopustags\fP refuses to overwrite an already existent file. Use
-this option to allow that.
+By default, \fBopustags\fP refuses to overwrite an already-existent file.
+Use \fB-y\fP to allow overwriting.
+Note that this option is not needed when the output is a special file like \fI/dev/null\fP.
 .TP
-.B \-d, \-\-delete \fIFIELD\fP
-Delete all the tags whose field name is \fIFIELD\fP. They may be several one of them, though usually
-there is only one of each type.
+.B \-d, \-\-delete \fIFIELD[=VALUE]\fP
+If value is not specified, delete all the tags whose field name is \fIFIELD\fP.
+Otherwise, delete all the comments whose field name is \fIFIELD\fP and value is \fIVALUE\fP.
+In both cases, the field names are case-insensitive, and expected to be ASCII.
 .TP
 .B \-a, \-\-add \fIFIELD=VALUE\fP
 Add a tag. Note that multiple tags with the same field name are perfectly acceptable, so you can add
@@ -87,8 +97,19 @@ Delete all the previously existing tags.
 Sets the tags from scratch.
 All the original tags are deleted and new ones are read from standard input.
 Each line must specify a \fIFIELD=VALUE\fP pair and be separated with line feeds.
-Invalid lines are skipped and cause a warning to be issued. Blank lines are ignored.
-This mode could be useful for batch processing tags through an utility like \fBsed\fP.
+Blank lines and lines starting with \fI#\fP are ignored.
+.TP
+.B \-e, \-\-edit
+Edit tags interactively by spawning the program specified by the EDITOR
+environment variable. The allowed format is the same as \fB--set-all\fP.
+If TERM and VISUAL are set, VISUAL takes precedence over EDITOR.
+.TP
+.B \-\-raw
+OpusTags metadata should always be encoded in UTF-8, as per RFC 7845. However, some files may be
+corrupted or possibly even contain intentional binary data. In that case, --raw lets you edit that
+kind of binary data without ensuring the validity of the tags encoding. This option may also be
+useful when your system encoding is different from UTF-8 and you wish to preserve the full UTF-8
+character set even though your system cannot display it.
 .SH EXAMPLES
 .PP
 List all the tags in file foo.opus:
@@ -107,10 +128,29 @@ Remove the previously existing ARTIST tags and add the two X and Y ARTIST tags,
 tags without writing them to the Opus file:
 .PP
 	opustags in.opus --add ARTIST=X --add ARTIST=Y --delete ARTIST
-.SH SEE ALSO
-.BR vorbiscomment (1),
-.BR sed (1)
+.PP
+Edit tags interactively in Vim:
+.PP
+	EDITOR=vim opustags --in-place --edit file.opus
+.SH CAVEATS
+.PP
+\fBopustags\fP currently has the following limitations:
+.IP \[bu]
+The total size of all tags cannot exceed 64 kB, the maximum size of one Ogg page.
+.IP \[bu]
+Multiplexed streams are not supported.
+.IP \[bu]
+Newlines inside tags are not supported by `--set-all`.
+.IP \[bu]
+Newlines and control characters are not escaped when printing tags.
+.PP
+Internally, the OpusTags packet in an Ogg Opus file may contain extra arbitrary binary data after
+the comments.  This block of data is currently not editable, but is always preserved. The same
+applies for the vendor string.
+.PP
+If you need a feature not currently supported, feel free to open an issue or send an email with your
+use case.
 .SH AUTHOR
-Frédéric Mangano-Tarumi <fmang@mg0.fr>
+Frédéric Mangano-Tarumi <fmang+opustags@mg0.fr>
 .PP
 Report bugs at <https://github.com/fmang/opustags/issues>

src/cli.cc

@@ -4,38 +4,43 @@
  *
  * Provide all the features of the opustags executable from a C++ API. The main point of separating
  * this module from the main one is to allow easy testing.
- *
- * \todo Use a safer temporary file name for in-place editing, like tmpnam.
- * \todo Abort editing with --set-all if one comment is invalid?
  */
 
-#include <config.h>
 #include <opustags.h>
 
+#include <errno.h>
 #include <getopt.h>
 #include <limits.h>
+#include <stdlib.h>
 #include <string.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
-static const char* version = PROJECT_NAME " version " PROJECT_VERSION "\n";
+using namespace std::literals::string_literals;
+
+static const char help_message[] =
+PROJECT_NAME " version " PROJECT_VERSION
+R"raw(
 
-static const char* usage = 1 + R"raw(
 Usage: opustags --help
        opustags [OPTIONS] FILE
+       opustags OPTIONS -i FILE...
        opustags OPTIONS FILE -o FILE
-)raw";
 
-static const char* help = 1 + R"raw(
 Options:
-  -h, --help              print this help
-  -o, --output FILE       set the output file
-  -i, --in-place          overwrite the input file instead of writing a different output file
-  -y, --overwrite         overwrite the output file if it already exists
-  -a, --add FIELD=VALUE   add a comment
-  -d, --delete FIELD      delete all previously existing comments of a specific type
-  -D, --delete-all        delete all the previously existing comments
-  -s, --set FIELD=VALUE   replace a comment (shorthand for --delete FIELD --add FIELD=VALUE)
-  -S, --set-all           replace all the comments with the ones read from standard input
+  -h, --help                    print this help
+  -o, --output FILE             specify the output file
+  -i, --in-place                overwrite the input files
+  -y, --overwrite               overwrite the output file if it already exists
+  -a, --add FIELD=VALUE         add a comment
+  -d, --delete FIELD[=VALUE]    delete previously existing comments
+  -D, --delete-all              delete all the previously existing comments
+  -s, --set FIELD=VALUE         replace a comment
+  -S, --set-all                 import comments from standard input
+  -e, --edit                    edit tags interactively in VISUAL/EDITOR
+  --raw                         disable encoding conversion
+
+See the man page for extensive documentation.
 )raw";
 
 static struct option getopt_options[] = {
@@ -48,115 +53,174 @@ static struct option getopt_options[] = {
 	{"set", required_argument, 0, 's'},
 	{"delete-all", no_argument, 0, 'D'},
 	{"set-all", no_argument, 0, 'S'},
+	{"edit", no_argument, 0, 'e'},
+	{"raw", no_argument, 0, 'r'},
 	{NULL, 0, 0, 0}
 };
 
-ot::status ot::process_options(int argc, char** argv, ot::options& opt)
+ot::status ot::parse_options(int argc, char** argv, ot::options& opt, FILE* comments_input)
 {
-	if (argc == 1) {
-		fputs(version, stdout);
-		fputs(usage, stdout);
-		return st::exit_now;
-	}
+	static ot::encoding_converter to_utf8("", "UTF-8");
+	std::string utf8;
+	const char* equal;
+	ot::status rc;
+	bool set_all = false;
+	opt = {};
+	if (argc == 1)
+		return {st::bad_arguments, "No arguments specified. Use -h for help."};
 	int c;
-	while ((c = getopt_long(argc, argv, "ho:i::yd:a:s:DS", getopt_options, NULL)) != -1) {
+	optind = 0;
+	while ((c = getopt_long(argc, argv, ":ho:iyd:a:s:DSe", getopt_options, NULL)) != -1) {
 		switch (c) {
 		case 'h':
 			opt.print_help = true;
 			break;
 		case 'o':
+			if (opt.path_out)
+				return {st::bad_arguments, "Cannot specify --output more than once."};
 			opt.path_out = optarg;
-			if (opt.path_out.empty()) {
-				fputs("output's file path cannot be empty\n", stderr);
-				return st::bad_arguments;
-			}
 			break;
 		case 'i':
-			opt.inplace = optarg == nullptr ? ".otmp" : optarg;
-			if (strcmp(opt.inplace, "") == 0) {
-				fputs("the in-place suffix cannot be empty\n", stderr);
-				return st::bad_arguments;
-			}
+			opt.in_place = true;
+			opt.overwrite = true;
 			break;
 		case 'y':
 			opt.overwrite = true;
 			break;
 		case 'd':
-			if (strchr(optarg, '=') != nullptr) {
-				fprintf(stderr, "invalid field name: '%s'\n", optarg);
-				return st::bad_arguments;
-			}
 			opt.to_delete.emplace_back(optarg);
 			break;
 		case 'a':
 		case 's':
-			if (strchr(optarg, '=') == NULL) {
-				fprintf(stderr, "invalid comment: '%s'\n", optarg);
-				return st::bad_arguments;
-			}
-			opt.to_add.emplace_back(optarg);
+			equal = strchr(optarg, '=');
+			if (equal == nullptr)
+				return {st::bad_arguments, "Comment does not contain an equal sign: "s + optarg + "."};
 			if (c == 's')
-				opt.to_delete.emplace_back(optarg);
+				opt.to_delete.emplace_back(optarg, equal - optarg);
+			opt.to_add.emplace_back(optarg);
 			break;
 		case 'S':
-			opt.set_all = true;
-			/* fall through */
+			opt.delete_all = true;
+			set_all = true;
+			break;
 		case 'D':
 			opt.delete_all = true;
 			break;
+		case 'e':
+			opt.edit_interactively = true;
+			break;
+		case 'r':
+			opt.raw = true;
+			break;
+		case ':':
+			return {st::bad_arguments,
+			        "Missing value for option '"s + argv[optind - 1] + "'."};
 		default:
-			/* getopt printed a message */
-			return st::bad_arguments;
+			return {st::bad_arguments, "Unrecognized option '" +
+			        (optopt ? "-"s + static_cast<char>(optopt) : argv[optind - 1]) + "'."};
 		}
 	}
-	if (opt.print_help) {
-		puts(version);
-		puts(usage);
-		puts(help);
-		puts("See the man page for extensive documentation.");
-		return st::exit_now;
+	if (opt.print_help)
+		return st::ok;
+
+	// All non-option arguments are input files.
+	bool stdin_as_input = false;
+	for (int i = optind; i < argc; i++) {
+		stdin_as_input = stdin_as_input || strcmp(argv[i], "-") == 0;
+		opt.paths_in.emplace_back(argv[i]);
 	}
-	if (optind != argc - 1) {
-		fputs("exactly one input file must be specified\n", stderr);
-		return st::bad_arguments;
-	}
-	opt.path_in = argv[optind];
-	if (opt.path_in.empty()) {
-		fputs("input's file path cannot be empty\n", stderr);
-		return st::bad_arguments;
-	}
-	if (opt.inplace != nullptr) {
-		if (!opt.path_out.empty()) {
-			fputs("cannot combine --in-place and --output\n", stderr);
-			return st::bad_arguments;
+
+	// Convert arguments to UTF-8.
+	if (!opt.raw) {
+		for (std::list<std::string>* args : { &opt.to_add, &opt.to_delete }) {
+			for (std::string& arg : *args) {
+				rc = to_utf8(arg, utf8);
+				if (rc != ot::st::ok)
+					return {st::bad_arguments, "Could not encode argument into UTF-8: " + rc.message};
+				arg = std::move(utf8);
+			}
 		}
-		opt.path_out = opt.path_in + opt.inplace;
 	}
-	if (opt.path_in == "-" && opt.set_all) {
-		fputs("can't open standard input for input when --set-all is specified\n", stderr);
-		return st::bad_arguments;
-	}
-	if (opt.path_in == "-" && opt.inplace) {
-		fputs("cannot modify standard input in-place\n", stderr);
-		return st::bad_arguments;
+
+	if (opt.in_place && opt.path_out)
+		return {st::bad_arguments, "Cannot combine --in-place and --output."};
+
+	if (opt.in_place && stdin_as_input)
+		return {st::bad_arguments, "Cannot modify standard input in place."};
+
+	if ((!opt.in_place || opt.edit_interactively) && opt.paths_in.size() != 1)
+		return {st::bad_arguments, "Exactly one input file must be specified."};
+
+	if (set_all && stdin_as_input)
+		return {st::bad_arguments, "Cannot use standard input as input file when --set-all is specified."};
+
+	if (opt.edit_interactively && (stdin_as_input || opt.path_out == "-"))
+		return {st::bad_arguments, "Cannot edit interactively when standard input or standard output are already used."};
+
+	if (opt.edit_interactively && !opt.path_out.has_value() && !opt.in_place)
+		return {st::bad_arguments, "Cannot edit interactively when no output is specified."};
+
+	if (opt.edit_interactively && (opt.delete_all || !opt.to_add.empty() || !opt.to_delete.empty()))
+		return {st::bad_arguments, "Cannot mix --edit with -adDsS."};
+
+	if (set_all) {
+		// Read comments from stdin and prepend them to opt.to_add.
+		std::list<std::string> comments;
+		auto rc = read_comments(comments_input, comments, opt.raw);
+		if (rc != st::ok)
+			return rc;
+		opt.to_add.splice(opt.to_add.begin(), std::move(comments));
 	}
 	return st::ok;
 }
 
 /**
- * \todo Escape new lines.
+ * \todo Find a way to support new lines such that they can be read back by #read_comment without
+ *       ambiguity. We could add a raw mode and separate comments with a \0, or escape control
+ *       characters with a backslash, but we should also preserve compatibiltity with potential
+ *       callers that don’t escape backslashes. Maybe add options to select a mode between simple,
+ *       raw, and escaped.
  */
-void ot::print_comments(const std::list<std::string>& comments, FILE* output)
+ot::status ot::print_comments(const std::list<std::string>& comments, FILE* output, bool raw)
 {
-	for (const std::string& comment : comments) {
-		fwrite(comment.data(), 1, comment.size(), output);
-		puts("");
+	static ot::encoding_converter from_utf8("UTF-8", "");
+	std::string local;
+	bool has_newline = false;
+	bool has_control = false;
+	for (const std::string& utf8_comment : comments) {
+		const std::string* comment;
+		// Convert the comment from UTF-8 to the system encoding if relevant.
+		if (raw) {
+			comment = &utf8_comment;
+		} else {
+			ot::status rc = from_utf8(utf8_comment, local);
+			comment = &local;
+			if (rc != ot::st::ok) {
+				rc.message += " See --raw.";
+				return rc;
+			}
+		}
+
+		for (unsigned char c : *comment) {
+			if (c == '\n')
+				has_newline = true;
+			else if (c < 0x20)
+				has_control = true;
+		}
+		fwrite(comment->data(), 1, comment->size(), output);
+		putc('\n', output);
 	}
+	if (has_newline)
+		fputs("warning: Some tags contain unsupported newline characters.\n", stderr);
+	if (has_control)
+		fputs("warning: Some tags contain control characters.\n", stderr);
+	return st::ok;
 }
 
-std::list<std::string> ot::read_comments(FILE* input)
+ot::status ot::read_comments(FILE* input, std::list<std::string>& comments, bool raw)
 {
-	std::list<std::string> comments;
+	static ot::encoding_converter to_utf8("", "UTF-8");
+	comments.clear();
 	char* line = nullptr;
 	size_t buflen = 0;
 	ssize_t nread;
@@ -165,167 +229,291 @@ std::list<std::string> ot::read_comments(FILE* input)
 			--nread;
 		if (nread == 0)
 			continue;
-		if (memchr(line, '=', nread) == nullptr) {
-			fputs("warning: skipping malformed tag\n", stderr);
+		if (line[0] == '#') // comment
 			continue;
+		if (memchr(line, '=', nread) == nullptr) {
+			ot::status rc = {ot::st::error, "Malformed tag: " + std::string(line, nread)};
+			free(line);
+			return rc;
+		}
+		if (raw) {
+			comments.emplace_back(line, nread);
+		} else {
+			std::string utf8;
+			ot::status rc = to_utf8(std::string_view(line, nread), utf8);
+			if (rc == ot::st::ok) {
+				comments.emplace_back(std::move(utf8));
+			} else {
+				free(line);
+				return {ot::st::badly_encoded, "UTF-8 conversion error: " + rc.message};
+			}
 		}
-		comments.emplace_back(line, nread);
 	}
 	free(line);
-	return comments;
+	return ot::st::ok;
 }
 
-/**
- * Parse the packet as an OpusTags comment header, apply the user's modifications, and write the new
- * packet to the writer.
- */
-static ot::status process_tags(const ogg_packet& packet, const ot::options& opt, ot::ogg_writer* writer)
+void ot::delete_comments(std::list<std::string>& comments, const std::string& selector)
 {
-	ot::opus_tags tags;
-	ot::status rc = ot::parse_tags(packet, tags);
-	if (rc != ot::st::ok)
-		return rc;
+	auto name = selector.data();
+	auto equal = selector.find('=');
+	auto value = (equal == std::string::npos ? nullptr : name + equal + 1);
+	auto name_len = value ? equal : selector.size();
+	auto value_len = value ? selector.size() - equal - 1 : 0;
+	auto it = comments.begin(), end = comments.end();
+	while (it != end) {
+		auto current = it++;
+		bool name_match = current->size() > name_len + 1 &&
+		                  (*current)[name_len] == '=' &&
+		                  strncasecmp(current->data(), name, name_len) == 0;
+		if (!name_match)
+			continue;
+		bool value_match = value == nullptr ||
+		                   (current->size() == selector.size() &&
+		                    memcmp(current->data() + equal + 1, value, value_len) == 0);
+		if (value_match)
+			comments.erase(current);
+	}
+}
 
+/** Apply the modifications requested by the user to the opustags packet. */
+static ot::status edit_tags(ot::opus_tags& tags, const ot::options& opt)
+{
 	if (opt.delete_all) {
 		tags.comments.clear();
-	} else {
-		for (const std::string& name : opt.to_delete)
-			ot::delete_comments(tags, name.c_str());
+	} else for (const std::string& name : opt.to_delete) {
+		ot::delete_comments(tags.comments, name.c_str());
 	}
 
-	if (opt.set_all)
-		tags.comments = ot::read_comments(stdin);
 	for (const std::string& comment : opt.to_add)
 		tags.comments.emplace_back(comment);
 
-	if (writer) {
-		auto packet = ot::render_tags(tags);
-		return writer->write_packet(packet);
-	} else {
-		ot::print_comments(tags.comments, stdout);
-		return ot::st::ok;
-	}
+	return ot::st::ok;
 }
 
-ot::status ot::process(ogg_reader& reader, ogg_writer* writer, const ot::options &opt)
+/** Spawn VISUAL or EDITOR to edit the given tags. */
+static ot::status edit_tags_interactively(ot::opus_tags& tags, const std::optional<std::string>& base_path, bool raw)
 {
-	int packet_count = 0;
-	for (;;) {
-		// Read the next page.
-		ot::status rc = reader.read_page();
-		if (rc == ot::st::end_of_stream)
-			break;
-		else if (rc != ot::st::ok)
-			return rc;
-		// Short-circuit when the relevant packets have been read.
-		if (packet_count >= 2 && writer) {
-			if ((rc = writer->write_page(reader.page)) != ot::st::ok)
-				return rc;
-			continue;
-		}
-		auto serialno = ogg_page_serialno(&reader.page);
-		if (writer && (rc = writer->prepare_stream(serialno)) != ot::st::ok)
-			return rc;
-		// Read all the packets.
-		for (;;) {
-			rc = reader.read_packet();
-			if (rc == ot::st::end_of_page)
-				break;
-			else if (rc != ot::st::ok)
-				return rc;
-			packet_count++;
-			if (packet_count == 1) { // Identification header
-				rc = ot::validate_identification_header(reader.packet);
-				if (rc != ot::st::ok)
-					return rc;
-			} else if (packet_count == 2) { // Comment header
-				rc = process_tags(reader.packet, opt, writer);
-				if (rc != ot::st::ok)
-					return rc;
-				if (!writer)
-					return ot::st::ok; /* nothing else to do */
-				else
-					continue; /* process_tags wrote the new packet */
-			}
-			if (writer && (rc = writer->write_packet(reader.packet)) != ot::st::ok)
-				return rc;
-		}
-		// Write the assembled page.
-		if (writer && (rc = writer->flush_page()) != ot::st::ok)
-			return rc;
+	const char* editor = nullptr;
+	if (getenv("TERM") != nullptr)
+		editor = getenv("VISUAL");
+	if (editor == nullptr) // without a terminal, or if VISUAL is unset
+		editor = getenv("EDITOR");
+	if (editor == nullptr)
+		return {ot::st::error,
+		        "No editor specified in environment variable VISUAL or EDITOR."};
+
+	// Building the temporary tags file.
+	ot::status rc;
+	std::string tags_path = base_path.value_or("tags") + ".XXXXXX.opustags";
+	int fd = mkstemps(const_cast<char*>(tags_path.data()), 9);
+	ot::file tags_file;
+	if (fd == -1 || (tags_file = fdopen(fd, "w")) == nullptr)
+		return {ot::st::standard_error,
+		        "Could not open '" + tags_path + "': " + strerror(errno)};
+	if ((rc = ot::print_comments(tags.comments, tags_file.get(), raw)) != ot::st::ok)
+		return rc;
+	tags_file.reset();
+
+	// Spawn the editor, and watch the modification timestamps.
+	timespec before, after;
+	if ((rc = ot::get_file_timestamp(tags_path.c_str(), before)) != ot::st::ok)
+		return rc;
+	ot::status editor_rc = ot::run_editor(editor, tags_path);
+	if ((rc = ot::get_file_timestamp(tags_path.c_str(), after)) != ot::st::ok)
+		return rc; // probably because the file was deleted
+	bool modified = (before.tv_sec != after.tv_sec || before.tv_nsec != after.tv_nsec);
+	if (editor_rc != ot::st::ok) {
+		if (modified)
+			fprintf(stderr, "warning: Leaving %s on the disk.\n", tags_path.c_str());
+		else
+			remove(tags_path.c_str());
+		return editor_rc;
+	} else if (!modified) {
+		remove(tags_path.c_str());
+		fputs("Cancelling edition because the tags file was not modified.\n", stderr);
+		return ot::st::cancel;
 	}
-	if (packet_count < 2)
-		return {ot::st::fatal_error, "Expected at least 2 Ogg packets"};
+
+	// Applying the new tags.
+	tags_file = fopen(tags_path.c_str(), "re");
+	if (tags_file == nullptr)
+		return {ot::st::standard_error, "Error opening " + tags_path + ": " + strerror(errno)};
+	if ((rc = ot::read_comments(tags_file.get(), tags.comments, raw)) != ot::st::ok) {
+		fprintf(stderr, "warning: Leaving %s on the disk.\n", tags_path.c_str());
+		return rc;
+	}
+	tags_file.reset();
+
+	// Remove the temporary tags file only on success, because unlike the
+	// partial Ogg file that is irrecoverable, the edited tags file
+	// contains user data, so let’s leave users a chance to recover it.
+	remove(tags_path.c_str());
+
 	return ot::st::ok;
 }
 
 /**
- * Check if two filepaths point to the same file, after path canonicalization.
- * The path "-" is treated specially, meaning stdin for path_in and stdout for path_out.
+ * Main loop of opustags. Read the packets from the reader, and forwards them to the writer.
+ * Transform the OpusTags packet on the fly.
+ *
+ * The writer is optional. When writer is nullptr, opustags runs in read-only mode.
  */
-static bool same_file(const std::string& path_in, const std::string& path_out)
+static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const ot::options &opt)
 {
-	if (path_in == "-" || path_out == "-")
-		return false;
-	char canon_in[PATH_MAX+1], canon_out[PATH_MAX+1];
-	if (realpath(path_in.c_str(), canon_in) && realpath(path_out.c_str(), canon_out)) {
-		return (strcmp(canon_in, canon_out) == 0);
+	bool focused = false; /*< the stream on which we operate is defined */
+	int focused_serialno; /*< when focused, the serialno of the focused stream */
+	int absolute_page_no = -1; /*< page number in the physical stream, not logical */
+	for (;;) {
+		ot::status rc = reader.next_page();
+		if (rc == ot::st::end_of_stream)
+			break;
+		else if (rc == ot::st::bad_stream && absolute_page_no == -1)
+			return {ot::st::bad_stream, "Input is not a valid Ogg file."};
+		else if (rc != ot::st::ok)
+			return rc;
+		++absolute_page_no;
+		auto serialno = ogg_page_serialno(&reader.page);
+		auto pageno = ogg_page_pageno(&reader.page);
+		if (!focused) {
+			focused = true;
+			focused_serialno = serialno;
+		} else if (serialno != focused_serialno) {
+			/** \todo Support mixed streams. */
+			return {ot::st::error, "Muxed streams are not supported yet."};
+		}
+		if (absolute_page_no == 0) { // Identification header
+			if (!ot::is_opus_stream(reader.page))
+				return {ot::st::error, "Not an Opus stream."};
+			if (writer) {
+				rc = writer->write_page(reader.page);
+				if (rc != ot::st::ok)
+					return rc;
+			}
+		} else if (absolute_page_no == 1) { // Comment header
+			ot::opus_tags tags;
+			rc = reader.process_header_packet(
+				[&tags](ogg_packet& p) { return ot::parse_tags(p, tags); });
+			if (rc != ot::st::ok)
+				return rc;
+			if ((rc = edit_tags(tags, opt)) != ot::st::ok)
+				return rc;
+			if (writer) {
+				if (opt.edit_interactively) {
+					fflush(writer->file); // flush before calling the subprocess
+					if ((rc = edit_tags_interactively(tags, writer->path, opt.raw)) != ot::st::ok)
+						return rc;
+				}
+				auto packet = ot::render_tags(tags);
+				rc = writer->write_header_packet(serialno, pageno, packet);
+				if (rc != ot::st::ok)
+					return rc;
+			} else {
+				if ((rc = ot::print_comments(tags.comments, stdout, opt.raw)) != ot::st::ok)
+					return rc;
+				break;
+			}
+		} else {
+			if (writer && (rc = writer->write_page(reader.page)) != ot::st::ok)
+				return rc;
+		}
 	}
-	return false;
-}
-
-ot::status ot::run(ot::options& opt)
-{
-	if (!opt.path_out.empty() && same_file(opt.path_in, opt.path_out))
-		return {ot::st::fatal_error, "Input and output files are the same"};
-
-	ot::file input;
-	if (opt.path_in == "-") {
-		input = stdin;
-	} else {
-		input = fopen(opt.path_in.c_str(), "r");
-		if (input == nullptr)
-			return {ot::st::standard_error,
-			        "Could not open '" + opt.path_in + "' for reading: " + strerror(errno)};
-	}
-
-	ot::file output;
-	if (opt.path_out == "-") {
-		output.reset(stdout);
-	} else if (!opt.path_out.empty()) {
-		if (!opt.overwrite && access(opt.path_out.c_str(), F_OK) == 0)
-			return {ot::st::fatal_error,
-			        "'" + opt.path_out + "' already exists (use -y to overwrite)"};
-		output = fopen(opt.path_out.c_str(), "w");
-		if (output == nullptr)
-			return {ot::st::standard_error,
-				"Could not open '" + opt.path_out + "' for writing: " + strerror(errno)};
-	}
-
-	ot::status rc;
-	{
-		ot::ogg_reader reader(input.get());
-		std::unique_ptr<ot::ogg_writer> writer;
-		if (output != nullptr)
-			writer = std::make_unique<ot::ogg_writer>(output.get());
-		rc = process(reader, writer.get(), opt);
-		/* delete reader and writer before closing the files */
-	}
-
-	input.reset();
-	output.reset();
-
-	if (rc != ot::st::ok) {
-		if (!opt.path_out.empty() && opt.path_out != "-")
-			remove(opt.path_out.c_str());
-		return rc;
-	}
-
-	if (opt.inplace) {
-		if (rename(opt.path_out.c_str(), opt.path_in.c_str()) == -1)
-			return {ot::st::fatal_error,
-			        "Could not move the result to '" + opt.path_in + "': " + strerror(errno)};
-	}
-
+	if (absolute_page_no < 1)
+		return {ot::st::error, "Expected at least 2 Ogg pages."};
 	return ot::st::ok;
 }
+
+static ot::status run_single(const ot::options& opt, const std::string& path_in, const std::optional<std::string>& path_out)
+{
+	ot::file input;
+	if (path_in == "-")
+		input = stdin;
+	else if ((input = fopen(path_in.c_str(), "re")) == nullptr)
+		return {ot::st::standard_error,
+		        "Could not open '" + path_in + "' for reading: " + strerror(errno)};
+	ot::ogg_reader reader(input.get());
+
+	/* Read-only mode. */
+	if (!path_out)
+		return process(reader, nullptr, opt);
+
+	/* Read-write mode.
+	 *
+	 * The output pointer is set to one of:
+	 *  - stdout for "-",
+	 *  - final_output.get() for special files like /dev/null,
+	 *  - temporary_output.get() for regular files.
+	 *
+	 * We use a temporary output file for the following reasons:
+	 *  1. A partial .opus output would be seen by softwares like media players, but a .part
+	 *     (for partial) won’t.
+	 *  2. If the process crashes badly, or the power cuts off, we don't want to leave a partial
+	 *     file at the final location. The temporary file is going to remain though.
+	 *  3. If we're overwriting a regular file, we'd rather avoid wiping its content before we
+	 *     even started reading the input file. That way, the original file is always preserved
+	 *     on error or crash.
+	 *  4. It is necessary for in-place editing. We can't reliably open the same file as both
+	 *     input and output.
+	 */
+
+	FILE* output = nullptr;
+	ot::partial_file temporary_output;
+	ot::file final_output;
+
+	ot::status rc = ot::st::ok;
+	struct stat output_info;
+	if (path_out == "-") {
+		output = stdout;
+	} else if (stat(path_out->c_str(), &output_info) == 0) {
+		/* The output file exists. */
+		if (!S_ISREG(output_info.st_mode)) {
+			/* Special files are opened for writing directly. */
+			if ((final_output = fopen(path_out->c_str(), "we")) == nullptr)
+				rc = {ot::st::standard_error,
+				      "Could not open '" + path_out.value() + "' for writing: " +
+				      strerror(errno)};
+			output = final_output.get();
+		} else if (opt.overwrite) {
+			rc = temporary_output.open(path_out->c_str());
+			output = temporary_output.get();
+		} else {
+			rc = {ot::st::error,
+			      "'" + path_out.value() + "' already exists. Use -y to overwrite."};
+		}
+	} else if (errno == ENOENT) {
+		rc = temporary_output.open(path_out->c_str());
+		output = temporary_output.get();
+	} else {
+		rc = {ot::st::error,
+		      "Could not identify '" + path_in + "': " + strerror(errno)};
+	}
+	if (rc != ot::st::ok)
+		return rc;
+
+	ot::ogg_writer writer(output);
+	writer.path = path_out;
+	rc = process(reader, &writer, opt);
+	if (rc == ot::st::ok)
+		rc = temporary_output.commit();
+
+	return rc;
+}
+
+ot::status ot::run(const ot::options& opt)
+{
+	if (opt.print_help) {
+		fputs(help_message, stdout);
+		return st::ok;
+	}
+
+	ot::status global_rc = st::ok;
+	for (const auto& path_in : opt.paths_in) {
+		ot::status rc = run_single(opt, path_in, opt.in_place ? path_in : opt.path_out);
+		if (rc != st::ok) {
+			global_rc = st::error;
+			if (!rc.message.empty())
+				fprintf(stderr, "%s: error: %s\n", path_in.c_str(), rc.message.c_str());
+		}
+	}
+	return global_rc;
+}

src/config.h.in

@@ -1,2 +1,7 @@
 #cmakedefine PROJECT_NAME "@PROJECT_NAME@"
 #cmakedefine PROJECT_VERSION "@PROJECT_VERSION@"
+
+#cmakedefine HAVE_ENDIAN_H @HAVE_ENDIAN_H@
+#cmakedefine HAVE_SYS_ENDIAN_H @HAVE_SYS_ENDIAN_H@
+#cmakedefine HAVE_STAT_ST_MTIM @HAVE_STAT_ST_MTIM@
+#cmakedefine HAVE_STAT_ST_MTIMESPEC @HAVE_STAT_ST_MTIMESPEC@

src/ogg.cc

@@ -10,77 +10,72 @@
 
 #include <opustags.h>
 
+#include <errno.h>
 #include <string.h>
 
 using namespace std::literals::string_literals;
 
-ot::ogg_reader::ogg_reader(FILE* input)
-	: file(input)
+bool ot::is_opus_stream(const ogg_page& identification_header)
 {
-	ogg_sync_init(&sync);
+	if (ogg_page_bos(&identification_header) == 0)
+		return false;
+	if (identification_header.body_len < 8)
+		return false;
+	return (memcmp(identification_header.body, "OpusHead", 8) == 0);
 }
 
-ot::ogg_reader::~ogg_reader()
+ot::status ot::ogg_reader::next_page()
 {
-	if (stream_ready)
-		ogg_stream_clear(&stream);
-	ogg_sync_clear(&sync);
-}
-
-ot::status ot::ogg_reader::read_page()
-{
-	while (ogg_sync_pageout(&sync, &page) != 1) {
-		if (feof(file))
-			return {st::end_of_stream, "End of stream was reached"};
+	int rc;
+	while ((rc = ogg_sync_pageout(&sync, &page)) != 1) {
+		if (rc == -1)
+			return {st::bad_stream, "Unsynced data in stream."};
+		if (ogg_sync_check(&sync) != 0)
+			return {st::libogg_error, "ogg_sync_check signalled an error."};
+		if (feof(file)) {
+			if (sync.fill != sync.returned)
+				return {st::bad_stream, "Unsynced data at end of stream."};
+			return {st::end_of_stream, "End of stream was reached."};
+		}
 		char* buf = ogg_sync_buffer(&sync, 65536);
 		if (buf == nullptr)
-			return {st::libogg_error, "ogg_sync_buffer failed"};
+			return {st::libogg_error, "ogg_sync_buffer failed."};
 		size_t len = fread(buf, 1, 65536, file);
 		if (ferror(file))
 			return {st::standard_error, "fread error: "s + strerror(errno)};
 		if (ogg_sync_wrote(&sync, len) != 0)
-			return {st::libogg_error, "ogg_sync_wrote failed"};
-		if (ogg_sync_check(&sync) != 0)
-			return {st::libogg_error, "ogg_sync_check failed"};
+			return {st::libogg_error, "ogg_sync_wrote failed."};
 	}
-	/* at this point, we've got a good page */
-	if (!stream_ready) {
-		if (ogg_stream_init(&stream, ogg_page_serialno(&page)) != 0)
-			return {st::libogg_error, "ogg_stream_init failed"};
-		stream_ready = true;
-	}
-	stream_in_sync = false;
 	return st::ok;
 }
 
-ot::status ot::ogg_reader::read_packet()
+ot::status ot::ogg_reader::process_header_packet(const std::function<status(ogg_packet&)>& f)
 {
-	if (!stream_ready)
-		return {st::stream_not_ready, "Stream was not initialized"};
-	if (!stream_in_sync) {
-		if (ogg_stream_pagein(&stream, &page) != 0)
-			return {st::libogg_error, "ogg_stream_pagein failed"};
-		stream_in_sync = true;
-	}
+	if (ogg_page_continued(&page))
+		return {ot::st::error, "Unexpected continued header page."};
+	ogg_logical_stream stream(ogg_page_serialno(&page));
+	stream.pageno = ogg_page_pageno(&page);
+	if (ogg_stream_pagein(&stream, &page) != 0)
+		return {st::libogg_error, "ogg_stream_pagein failed."};
+	ogg_packet packet;
 	int rc = ogg_stream_packetout(&stream, &packet);
-	if (rc == 1)
-		return st::ok;
-	else if (rc == 0 && ogg_stream_check(&stream) == 0)
-		return {st::end_of_page, "End of page was reached"};
-	else
-		return {st::libogg_error, "ogg_stream_packetout failed"};
-}
-
-ot::ogg_writer::ogg_writer(FILE* output)
-	: file(output)
-{
-	if (ogg_stream_init(&stream, 0) != 0)
-		throw std::bad_alloc();
-}
-
-ot::ogg_writer::~ogg_writer()
-{
-	ogg_stream_clear(&stream);
+	if (ogg_stream_check(&stream) != 0 || rc == -1)
+		return {ot::st::libogg_error, "ogg_stream_packetout failed."};
+	else if (rc == 0)
+		return {ot::st::error,
+		        "Reading header packets spanning multiple pages are not yet supported. "
+		        "Please file an issue to make your wish known."};
+	ot::status f_rc = f(packet);
+	if (f_rc != ot::st::ok)
+		return f_rc;
+	/* Ensure that there are no other segments left in the packet using the lacing state of the
+	 * stream. These are the relevant variables, as far as I understood them:
+	 *  - lacing_vals: extensible array containing the lacing values of the segments,
+	 *  - lacing_fill: number of elements in lacing_vals (not the capacity),
+	 *  - lacing_returned: index of the next segment to be processed. */
+	if (stream.lacing_returned != stream.lacing_fill)
+		return {ot::st::error, "Header page contains more than a single packet."};
+	return ot::st::ok;
 }
 
 ot::status ot::ogg_writer::write_page(const ogg_page& page)
@@ -96,29 +91,26 @@ ot::status ot::ogg_writer::write_page(const ogg_page& page)
 	return st::ok;
 }
 
-ot::status ot::ogg_writer::prepare_stream(long serialno)
-{
-	if (stream.serialno != serialno) {
-		if (ogg_stream_reset_serialno(&stream, serialno) != 0)
-			return {st::libogg_error, "ogg_stream_reset_serialno failed"};
-	}
-	return st::ok;
-}
-
-ot::status ot::ogg_writer::write_packet(const ogg_packet& packet)
-{
-	if (ogg_stream_packetin(&stream, const_cast<ogg_packet*>(&packet)) != 0)
-		return {st::libogg_error, "ogg_stream_packetin failed"};
-	else
-		return st::ok;
-}
-
-ot::status ot::ogg_writer::flush_page()
+ot::status ot::ogg_writer::write_header_packet(int serialno, int pageno, ogg_packet& packet)
 {
+	ogg_logical_stream stream(serialno);
+	stream.b_o_s = (pageno != 0);
+	stream.pageno = pageno;
+	if (ogg_stream_packetin(&stream, &packet) != 0)
+		return {ot::st::libogg_error, "ogg_stream_packetin failed"};
 	ogg_page page;
+	if (ogg_stream_flush(&stream, &page) != 0) {
+		ot::status rc = write_page(page);
+		if (rc != ot::st::ok)
+			return rc;
+	} else {
+		return {ot::st::libogg_error, "ogg_stream_flush failed"};
+	}
 	if (ogg_stream_flush(&stream, &page) != 0)
-		return write_page(page);
+		return {ot::st::error,
+		        "Writing header packets spanning multiple pages are not yet supported. "
+		        "Please file an issue to make your wish known."};
 	if (ogg_stream_check(&stream) != 0)
 		return {st::libogg_error, "ogg_stream_check failed"};
-	return st::ok; /* nothing was done */
+	return ot::st::ok;
 }

src/opus.cc

@@ -18,7 +18,6 @@
  *
  * \todo Validate that the vendor string and comments are valid UTF-8.
  * \todo Validate that field names are ASCII: 0x20 through 0x7D, 0x3D ('=') excluded.
- * \todo Field names are case insensitive, respect that.
  *
  */
 
@@ -26,29 +25,20 @@
 
 #include <string.h>
 
+#ifdef HAVE_ENDIAN_H
+#  include <endian.h>
+#endif
+
+#ifdef HAVE_SYS_ENDIAN_H
+#  include <sys/endian.h>
+#endif
+
 #ifdef __APPLE__
 #include <libkern/OSByteOrder.h>
 #define htole32(x) OSSwapHostToLittleInt32(x)
 #define le32toh(x) OSSwapLittleToHostInt32(x)
 #endif
 
-/**
- * \todo Validate more properties of the packet, like the sequence number.
- */
-ot::status ot::validate_identification_header(const ogg_packet& packet)
-{
-	if (packet.bytes < 8)
-		return {ot::st::cut_magic_number,
-		        "Identification header too short for the magic number"};
-	if (memcmp(packet.packet, "OpusHead", 8) != 0)
-		return {ot::st::bad_magic_number,
-		        "Identification header did not start with OpusHead"};
-	return ot::st::ok;
-}
-
-/**
- * \todo See if the packet's data could be casted more nicely into a string.
- */
 ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags)
 {
 	if (packet.bytes < 0)
@@ -135,29 +125,3 @@ ot::dynamic_ogg_packet ot::render_tags(const opus_tags& tags)
 
 	return op;
 }
-
-/**
- * \todo Make the field name case-insensitive?
- */
-static int match_field(const char *comment, uint32_t len, const char *field)
-{
-	size_t field_len;
-	for (field_len = 0; field[field_len] != '\0' && field[field_len] != '='; field_len++);
-	if (len <= field_len)
-		return 0;
-	if (comment[field_len] != '=')
-		return 0;
-	if (strncmp(comment, field, field_len) != 0)
-		return 0;
-	return 1;
-}
-
-void ot::delete_comments(opus_tags& tags, const char* field_name)
-{
-	auto it = tags.comments.begin(), end = tags.comments.end();
-	while (it != end) {
-		auto current = it++;
-		if (match_field(current->data(), current->size(), field_name))
-			tags.comments.erase(current);
-	}
-}

src/opustags.cc

@@ -7,29 +7,21 @@
 
 #include <opustags.h>
 
+#include <locale.h>
+
 /**
- * Main entry point to the opustags binary.
+ * Main function of the opustags binary.
  *
  * Does practically nothing but call the cli module.
  */
 int main(int argc, char** argv) {
-	ot::status rc;
+	setlocale(LC_ALL, "");
 	ot::options opt;
-	rc = process_options(argc, argv, opt);
-	if (rc == ot::st::exit_now) {
-		return EXIT_SUCCESS;
-	} else if (rc != ot::st::ok) {
-		if (!rc.message.empty())
-			fprintf(stderr, "error: %s\n", rc.message.c_str());
-		return EXIT_FAILURE;
-	}
+	ot::status rc = ot::parse_options(argc, argv, opt, stdin);
+	if (rc == ot::st::ok)
+		rc = ot::run(opt);
+	else if (!rc.message.empty())
+		fprintf(stderr, "error: %s\n", rc.message.c_str());
 
-	rc = run(opt);
-	if (rc != ot::st::ok && rc != ot::st::exit_now) {
-		if (!rc.message.empty())
-			fprintf(stderr, "error: %s\n", rc.message.c_str());
-		return EXIT_FAILURE;
-	}
-
-	return EXIT_SUCCESS;
+	return rc == ot::st::ok ? EXIT_SUCCESS : EXIT_FAILURE;
 }

src/opustags.h

@@ -5,6 +5,7 @@
  *
  * Let's have a quick tour around. The project is split into the following modules:
  *
+ * - The system module provides a few generic tools for interating with the system.
  * - The ogg module reads and writes Ogg files, letting you manipulate Ogg pages and packets.
  * - The opus module parses the contents of Ogg packets according to the Opus specifications.
  * - The cli module implements the main logic of the program.
@@ -23,12 +24,19 @@
 
 #pragma once
 
+#include <config.h>
+
+#include <iconv.h>
 #include <ogg/ogg.h>
 #include <stdio.h>
+#include <time.h>
 
+#include <functional>
 #include <list>
 #include <memory>
+#include <optional>
 #include <string>
+#include <string_view>
 #include <vector>
 
 namespace ot {
@@ -41,6 +49,9 @@ namespace ot {
  * have it explictly mentionned in their documentation. By default, a non-ok status should be
  * handled like an error.
  *
+ * Error codes do not need to be ultra specific, and are mainly used to report special conditions to
+ * the caller function. Ultimately, only the error message in the #status is shown to the user.
+ *
  * The cut error family means that the end of packet was reached when attempting to read the
  * overflowing value. For example, cut_comment_count means that after reading the vendor string,
  * less than 4 bytes were left in the packet.
@@ -48,12 +59,16 @@ namespace ot {
 enum class st {
 	/* Generic */
 	ok,
+	error,
+	standard_error, /**< Error raised by the C standard library. */
 	int_overflow,
-	standard_error,
+	cancel,
+	/* System */
+	badly_encoded,
+	child_process_failed,
 	/* Ogg */
+	bad_stream,
 	end_of_stream,
-	end_of_page,
-	stream_not_ready,
 	libogg_error,
 	/* Opus */
 	bad_magic_number,
@@ -65,8 +80,6 @@ enum class st {
 	cut_comment_data,
 	/* CLI */
 	bad_arguments,
-	exit_now, /**< The program should terminate successfully. */
-	fatal_error,
 };
 
 /**
@@ -75,6 +88,10 @@ enum class st {
  *
  * All the statuses except #st::ok should be accompanied with a relevant error message, in case it
  * propagates back to the main function and is shown to the user.
+ *
+ * \todo Instead of being returned, it could be thrown. Most of the error handling code just let the
+ *       status bubble. When we're confident about RAII, we're good to go. When we migrate, let's
+ *       start from main and adapt the functions top-down.
  */
 struct status {
 	status(st code = st::ok) : code(code) {}
@@ -84,6 +101,11 @@ struct status {
 	std::string message;
 };
 
+/***********************************************************************************************//**
+ * \defgroup system System
+ * \{
+ */
+
 /**
  * Smart auto-closing FILE* handle.
  *
@@ -93,32 +115,125 @@ struct file : std::unique_ptr<FILE, decltype(&fclose)> {
 	file(FILE* f = nullptr) : std::unique_ptr<FILE, decltype(&fclose)>(f, &fclose) {}
 };
 
+/**
+ * A partial file is a temporary file created to store the result of something. When it is complete,
+ * it is moved to a final destination. Open it with #open and then you can either #commit it to save
+ * it to its destination, or you can #abort to delete the temporary file. When the #partial_file
+ * object is destroyed, it deletes the currently opened temporary file, if any.
+ */
+class partial_file {
+public:
+	~partial_file() { abort(); }
+	/**
+	 * Open a temporary file meant to be moved to the specified destination file path. The
+	 * temporary file is created in the same directory as its destination in order to make the
+	 * final move operation instant.
+	 */
+	ot::status open(const char* destination);
+	/** Close then move the partial file to its final location. */
+	ot::status commit();
+	/** Delete the temporary file. */
+	void abort();
+	/** Get the underlying FILE* handle. */
+	FILE* get() { return file.get(); }
+	/** Get the name of the temporary file. */
+	const char* name() const { return file == nullptr ? nullptr : temporary_name.c_str(); }
+private:
+	std::string temporary_name;
+	std::string final_name;
+	ot::file file;
+};
+
+/** C++ wrapper for iconv. */
+class encoding_converter {
+public:
+	/**
+	 * Allocate the iconv conversion state, initializing the given source and destination
+	 * character encodings. If it's okay to have some information lost, make sure `to` ends with
+	 * "//TRANSLIT", otherwise the conversion will fail when a character cannot be represented
+	 * in the target encoding. See the documentation of iconv_open for details.
+	 */
+	encoding_converter(const char* from, const char* to);
+	~encoding_converter();
+	/**
+	 * Convert text using iconv. If the input sequence is invalid, return #st::badly_encoded and
+	 * abort the processing, leaving out in an undefined state.
+	 */
+	status operator()(std::string_view in, std::string& out);
+private:
+	iconv_t cd; /**< conversion descriptor */
+};
+
+/** Escape a string so that a POSIX shell interprets it as a single argument. */
+std::string shell_escape(std::string_view word);
+
+/**
+ * Execute the editor process specified in editor. Wait for the process to exit and
+ * return st::ok on success, or st::child_process_failed if it did not exit with 0.
+ *
+ * editor is passed unescaped to the shell, and may contain CLI options.
+ * path is the name of the file to edit, which will be passed as the last argument to editor.
+ */
+ot::status run_editor(std::string_view editor, std::string_view path);
+
+/**
+ * Return the specified path’s mtime, i.e. the last data modification
+ * timestamp.
+ */
+ot::status get_file_timestamp(const char* path, timespec& mtime);
+
+/** \} */
+
 /***********************************************************************************************//**
  * \defgroup ogg Ogg
  * \{
  */
 
 /**
- * Ogg reader, combining a FILE input, an ogg_sync_state reading the pages, and an ogg_stream_state
- * extracting the packets from the page.
+ * RAII-aware wrapper around libogg's ogg_stream_state. Though it handles automatic destruction, it
+ * does not prevent copying or implement move semantics correctly, so it's your responsibility to
+ * ensure these operations don't happen.
+ */
+struct ogg_logical_stream : ogg_stream_state {
+	ogg_logical_stream(int serialno) {
+		if (ogg_stream_init(this, serialno) != 0)
+			throw std::bad_alloc();
+	}
+	~ogg_logical_stream() {
+		ogg_stream_clear(this);
+	}
+};
+
+/**
+ * Identify the codec of a logical stream based on the first bytes of the first packet of the first
+ * page. For Opus, the first 8 bytes must be OpusHead. Any other signature is assumed to be another
+ * codec.
+ */
+bool is_opus_stream(const ogg_page& identification_header);
+
+/**
+ * Ogg reader, combining a FILE input, an ogg_sync_state reading the pages.
  *
  * Call #read_page repeatedly until #status::end_of_stream to consume the stream, and use #page to
- * check its content. To extract its packets, call #read_packet until #status::end_of_packet.
+ * check its content.
+ *
+ * \todo This class could be made more intuitive if it acted like an iterator, to be used like
+ *       `for (ogg_page& page : ogg_reader(input))`, but the prerequisite for this is the ability to
+ *       throw an exception on error.
  */
-class ogg_reader {
-public:
+struct ogg_reader {
 	/**
 	 * Initialize the reader with the given input file handle. The caller is responsible for
 	 * keeping the file handle alive, and to close it.
 	 */
-	ogg_reader(FILE* input);
+	ogg_reader(FILE* input) : file(input) { ogg_sync_init(&sync); }
 	/**
 	 * Clear all the internal memory allocated by libogg for the sync and stream state. The
 	 * page and the packet are owned by these states, so nothing to do with them.
 	 *
 	 * The input file is not closed.
 	 */
-	~ogg_reader();
+	~ogg_reader() { ogg_sync_clear(&sync); }
 	/**
 	 * Read the next page from the input file. The result, provided the status is #status::ok,
 	 * is made available in the #page field, is owned by the Ogg reader, and is valid until the
@@ -126,17 +241,16 @@ public:
 	 *
 	 * After the last page was read, return #status::end_of_stream.
 	 */
-	status read_page();
+	status next_page();
 	/**
-	 * Read the next available packet from the current #page. The packet is made available in
-	 * the #packet field.
+	 * Read the single packet contained in the last page read, assuming it's a header page, and
+	 * call the function f on it. This function has no side effect, and calling it twice on the
+	 * same page will read the same packet again.
 	 *
-	 * No packet can be read until a page has been loaded with #read_page. If that happens,
-	 * return #status::stream_not_ready.
-	 *
-	 * After the last packet was read, return #status::end_of_page.
+	 * It is currently limited to packets that fit on a single page, and should be later
+	 * extended to support packets spanning multiple pages.
 	 */
-	status read_packet();
+	status process_header_packet(const std::function<status(ogg_packet&)>& f);
 	/**
 	 * Current page from the sync state.
 	 *
@@ -144,14 +258,6 @@ public:
 	 * to ogg_sync_pageout, wrapped by #read_page.
 	 */
 	ogg_page page;
-	/**
-	 * Current packet from the stream state.
-	 *
-	 * Its memory is managed by libogg, inside the stream state, and is valid until the next
-	 * call to ogg_stream_packetout, wrapped by #read_packet.
-	 */
-	ogg_packet packet;
-private:
 	/**
 	 * The file is our source of binary data. It is not integrated to libogg, so we need to
 	 * handle it ourselves.
@@ -167,52 +273,20 @@ private:
 	 * are simply forwarded to the Ogg writer.
 	 */
 	ogg_sync_state sync;
-	/**
-	 * Indicates whether the stream has been initialized or not.
-	 *
-	 * To initialize it properly, we need the serialno of the stream, which is available only
-	 * after the first page was read.
-	 */
-	bool stream_ready = false;
-	/**
-	 * Indicates if the stream's last fed page is the current one.
-	 *
-	 * Its state is irrelevant if the stream is not ready.
-	 */
-	bool stream_in_sync;
-	/**
-	 * The stream layer receives pages and yields a sequence of packets.
-	 *
-	 * A single page may contain several packets, and a single packet may span on multiple
-	 * pages. The 2 packets we're interested in occupy whole pages though, in theory, but we'd
-	 * better ensure there are no extra packets anyway.
-	 *
-	 * After we've read OpusHead and OpusTags, we don't need the stream layer anymore.
-	 */
-	ogg_stream_state stream;
 };
 
 /**
  * An Ogg writer lets you write ogg_page objets to an output file, and assemble packets into pages.
  *
- * It has two modes of operations :
- *   1. call #write_page, or
- *   2. call #prepare_stream, then #write_packet one or more times, followed by #flush_page.
- *
- * You can switch between the two modes, but must not start writing packets and then pages without
- * flushing.
+ * Its packet writing facility is limited to writing single-page header packets, because that's all
+ * we need for opustags.
  */
-class ogg_writer {
-public:
+struct ogg_writer {
 	/**
 	 * Initialize the writer with the given output file handle. The caller is responsible for
 	 * keeping the file handle alive, and to close it.
 	 */
-	ogg_writer(FILE* output);
-	/**
-	 * Clears the stream state and any internal memory. Does not close the output file.
-	 */
-	~ogg_writer();
+	explicit ogg_writer(FILE* output) : file(output) {}
 	/**
 	 * Write a whole Ogg page into the output stream.
 	 *
@@ -220,43 +294,19 @@ public:
 	 */
 	status write_page(const ogg_page& page);
 	/**
-	 * Prepare the stream with the given Ogg serial number.
-	 *
-	 * If the stream is already configured with the right serial number, it doesn't do anything
-	 * and is cheap to call.
-	 *
-	 * If the stream contains unflushed packets, they will be lost.
+	 * Write a header packet and flush the page. Header packets are always placed alone on their
+	 * pages.
 	 */
-	status prepare_stream(long serialno);
-	/**
-	 * Add a packet to the current page under assembly.
-	 *
-	 * If the packet is coming from a different page, make sure the serial number fits by
-	 * calling #prepare_stream.
-	 *
-	 * When the page is complete, you should call #flush_page to finalize the page.
-	 *
-	 * You must not call #write_page after it, until you call #flush_page.
-	 */
-	status write_packet(const ogg_packet& packet);
-	/**
-	 * Write the page under assembly. Future calls to #write_packet will be written in a new
-	 * page.
-	 */
-	status flush_page();
-private:
-	/**
-	 * The stream state receives packets and generates pages.
-	 *
-	 * In our specific use case, we only need it to put the OpusHead and OpusTags packets into
-	 * their own pages. The other pages are naively written to the output stream.
-	 */
-	ogg_stream_state stream;
+	status write_header_packet(int serialno, int pageno, ogg_packet& packet);
 	/**
 	 * Output file. It should be opened in binary mode. We use it to write whole pages,
 	 * represented as a block of data and a length.
 	 */
 	FILE* file;
+	/**
+	 * Path to the output file.
+	 */
+	std::optional<std::string> path;
 };
 
 /**
@@ -293,16 +343,16 @@ private:
 struct opus_tags {
 	/**
 	 * OpusTags packets begin with a vendor string, meant to identify the implementation of the
-	 * encoder. It should be an arbitrary UTF-8 string.
+	 * encoder. It is expected to be an arbitrary UTF-8 string.
 	 */
 	std::string vendor;
 	/**
-	 * Comments. These are a list of string following the NAME=Value format.  A comment may also
-	 * be called a field, or a tag.
+	 * Comments are strings in the NAME=Value format. A comment may also be called a field, or a
+	 * tag.
 	 *
-	 * The field name in vorbis comment is case-insensitive and ASCII, while the value can be
-	 * any valid UTF-8 string. The specification is not too clear for Opus, but let's assume
-	 * it's the same.
+	 * The field name in vorbis comments is usually case-insensitive and ASCII, while the value
+	 * can be any valid UTF-8 string. The specification is not too clear for Opus, but let's
+	 * assume it's the same.
 	 */
 	std::list<std::string> comments;
 	/**
@@ -319,13 +369,6 @@ struct opus_tags {
 	std::string extra_data;
 };
 
-/**
- * Validate the content of the first packet of an Ogg stream to ensure it's a valid OpusHead.
- *
- * Returns #ot::status::ok on success, #ot::status::bad_identification_header on error.
- */
-status validate_identification_header(const ogg_packet& packet);
-
 /**
  * Read the given OpusTags packet and extract its content into an opus_tags object.
  *
@@ -338,11 +381,6 @@ status parse_tags(const ogg_packet& packet, opus_tags& tags);
  */
 dynamic_ogg_packet render_tags(const opus_tags& tags);
 
-/**
- * Remove all the comments whose field name is equal to the special one, case-sensitive.
- */
-void delete_comments(opus_tags& tags, const char* field_name);
-
 /** \} */
 
 /***********************************************************************************************//**
@@ -351,124 +389,127 @@ void delete_comments(opus_tags& tags, const char* field_name);
  */
 
 /**
- * Structured representation of the arguments to opustags.
+ * Structured representation of the command-line arguments to opustags.
  */
 struct options {
-	/**
-	 * Path to the input file. It cannot be empty. The special "-" string means stdin.
-	 *
-	 * This is the mandatory non-flagged parameter.
-	 */
-	std::string path_in;
-	/**
-	 * Path to the optional file. The special "-" string means stdout. When empty, opustags runs
-	 * in read-only mode.
-	 *
-	 * Option: --output
-	 */
-	std::string path_out;
-	/**
-	 * If null, in-place editing is disabled. Otherwise, it points to the suffix to add to the
-	 * file name.
-	 *
-	 * Option: --in-place
-	 */
-	const char* inplace = nullptr;
-	/**
-	 * List of field names to delete. `{"ARTIST"}` will delete *all* the comments `ARTIST=*`. It
-	 * is currently case-sensitive. When #delete_all is true, it becomes meaningless.
-	 *
-	 * #to_add takes precedence over #to_delete, so if the same comment appears in both lists,
-	 * the one in #to_delete applies only to the previously existing tags.
-	 *
-	 * \todo Consider making it case-insensitive.
-	 * \todo Allow values like `ARTIST=x` to delete only the ARTIST comment whose value is x.
-	 *
-	 * Option: --delete, --set
-	 */
-	std::vector<std::string> to_delete;
-	/**
-	 * List of comments to add, in the current system encoding. For exemple `TITLE=a b c`. They
-	 * must be valid.
-	 *
-	 * Options: --add, --set, --set-all
-	 */
-	std::vector<std::string> to_add;
-	/**
-	 * Delete all the existing comments.
-	 *
-	 * Option: --delete-all
-	 */
-	bool delete_all = false;
-	/**
-	 * Replace the previous comments by the ones supplied by the user.
-	 *
-	 * Read a list of comments from stdin and populate #to_add. Implies #delete_all. Further
-	 * comments may be added with the --add option.
-	 *
-	 * Option: --set-all
-	 */
-	bool set_all = false;
-	/**
-	 * By default, opustags won't overwrite the output file if it already exists.
-	 *
-	 * Option: --overwrite
-	 */
-	bool overwrite = false;
 	/**
 	 * When true, opustags prints a detailed help and exits. All the other options are ignored.
 	 *
 	 * Option: --help
 	 */
 	bool print_help = false;
+	/**
+	 * Paths to the input files. The special string "-" means stdin.
+	 *
+	 * At least one input file must be given. If `--in-place` is used,
+	 * more than one may be given.
+	 */
+	std::vector<std::string> paths_in;
+	/**
+	 * Optional path to output file. The special string "-" means stdout. For in-place
+	 * editing, the input file name is used. If no output file name is supplied, and
+	 * --in-place is not used, opustags runs in read-only mode.
+	 *
+	 * Options: --output, --in-place
+	 */
+	std::optional<std::string> path_out;
+	/**
+	 * By default, opustags won't overwrite the output file if it already exists. This can be
+	 * forced with --overwrite. It is also enabled by --in-place.
+	 *
+	 * Options: --overwrite, --in-place
+	 */
+	bool overwrite = false;
+	/**
+	 * Process files in-place.
+	 *
+	 * Options: --in-place
+	 */
+	bool in_place = false;
+	/**
+	 * Spawn EDITOR to edit tags interactively.
+	 *
+	 * stdin and stdout must be left free for the editor, so paths_in and
+	 * path_out can’t take `-`, and --set-all is not supported.
+	 *
+	 * Option: --edit
+	 */
+	bool edit_interactively = false;
+	/**
+	 * List of comments to delete. Each string is a selector according to the definition of
+	 * #delete_comments.
+	 *
+	 * When #delete_all is true, this option is meaningless.
+	 *
+	 * #to_add takes precedence over #to_delete, so if the same comment appears in both lists,
+	 * the one in #to_delete applies only to the previously existing tags.
+	 *
+	 * The strings are stored in UTF-8.
+	 *
+	 * Option: --delete, --set
+	 */
+	std::list<std::string> to_delete;
+	/**
+	 * Delete all the existing comments.
+	 *
+	 * Option: --delete-all, --set-all
+	 */
+	bool delete_all = false;
+	/**
+	 * List of comments to add, in the current system encoding. For exemple `TITLE=a b c`. They
+	 * must be valid.
+	 *
+	 * The strings are stored in UTF-8.
+	 *
+	 * Options: --add, --set, --set-all
+	 */
+	std::list<std::string> to_add;
+	/**
+	 * Disable encoding conversions. OpusTags are specified to always be encoded as UTF-8, but
+	 * if for some reason a specific file contains binary tags that someone would like to
+	 * extract and set as-is, encoding conversion would get in the way.
+	 */
+	bool raw = false;
 };
 
 /**
- * Process the command-line arguments.
+ * Parse the command-line arguments. Does not perform I/O related validations, but checks the
+ * consistency of its arguments. Comments are read if necessary from the given stream.
  *
- * This function does not perform I/O related validations, but checks the consistency of its
- * arguments.
- *
- * It returns one of :
- * - #ot::st::ok, meaning the process may continue normally.
- * - #ot::st::exit_now, meaning there is nothing to do and process should exit successfully.
- *   This happens when all the user wants is see the help or usage.
- * - #ot::st::bad_arguments, meaning the arguments were invalid and the process should exit with
- *   an error.
- *
- * Help messages are written on standard output, and error messages on standard error.
+ * On error, the state of the options structure is unspecified.
  */
-status process_options(int argc, char** argv, options& opt);
+status parse_options(int argc, char** argv, options& opt, FILE* comments);
 
 /**
- * Print all the comments, separated by line breaks. Since a comment may
- * contain line breaks, this output is not completely reliable, but it fits
- * most cases.
+ * Print all the comments, separated by line breaks. Since a comment may contain line breaks, this
+ * output is not completely reliable, but it fits most cases.
  *
- * The output generated is meant to be parseable by #ot::read_tags.
+ * The comments must be encoded in UTF-8, and are converted to the system locale when printed.
+ *
+ * The output generated is meant to be parseable by #ot::read_comments.
  */
-void print_comments(const std::list<std::string>& comments, FILE* output);
+status print_comments(const std::list<std::string>& comments, FILE* output, bool raw);
 
 /**
  * Parse the comments outputted by #ot::print_comments.
+ *
+ * The comments are converted from the system encoding to UTF-8, and returned as UTF-8.
  */
-std::list<std::string> read_comments(FILE* input);
+status read_comments(FILE* input, std::list<std::string>& comments, bool raw);
 
 /**
- * Main loop of opustags. Read the packets from the reader, and forwards them to the writer.
- * Transform the OpusTags packet on the fly.
+ * Remove all comments matching the specified selector, which may either be a field name or a
+ * NAME=VALUE pair. The field name is case-insensitive.
  *
- * The writer is optional. When writer is nullptr, opustags runs in read-only mode.
+ * The strings are all UTF-8.
  */
-status process(ogg_reader& reader, ogg_writer* writer, const options &opt);
+void delete_comments(std::list<std::string>& comments, const std::string& selector);
 
 /**
- * Open the input and output streams, then call #ot::process.
- *
- * This is the main entry point to the opustags program, and pretty much the same as calling
- * opustags from the command-line.
+ * Main entry point to the opustags program, and pretty much the same as calling opustags from the
+ * command-line.
  */
-status run(options& opt);
+status run(const options& opt);
 
 /** \} */
 

src/system.cc

@@ -0,0 +1,190 @@
+/**
+ * \file src/system.cc
+ * \ingroup system
+ *
+ * Provide a high-level interface to system-related features, like filesystem manipulations.
+ *
+ * Ideally, all OS-specific features should be grouped here.
+ *
+ * This modules shoumd not depend on any other opustags module.
+ */
+
+#include <opustags.h>
+
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+using namespace std::string_literals;
+
+ot::status ot::partial_file::open(const char* destination)
+{
+	abort();
+	final_name = destination;
+	temporary_name = final_name + ".XXXXXX.part";
+	int fd = mkstemps(const_cast<char*>(temporary_name.data()), 5);
+	if (fd == -1)
+		return {st::standard_error,
+		        "Could not create a partial file for '" + final_name + "': " +
+		        strerror(errno)};
+	file = fdopen(fd, "w");
+	if (file == nullptr)
+		return {st::standard_error,
+		        "Could not get the partial file handle to '" + temporary_name + "': " +
+		        strerror(errno)};
+	return st::ok;
+}
+
+static mode_t get_umask()
+{
+	// libc doesn’t seem to provide a way to get umask without changing it, so we need this workaround.
+	// https://www.gnu.org/software/libc/manual/html_node/Setting-Permissions.html
+	mode_t mask = umask(0);
+	umask(mask);
+	return mask;
+}
+
+/**
+ * Try reproducing the file permissions of file `source` onto file `dest`. If
+ * this fails for whatever reason, print a warning and leave the current
+ * permissions. When the source doesn’t exist, use the default file creation
+ * permissions according to umask.
+ */
+static void copy_permissions(const char* source, const char* dest)
+{
+	mode_t target_mode;
+	struct stat source_stat;
+	if (stat(source, &source_stat) == 0) {
+		// We could technically preserve a bit more than that but who
+		// would ever need S_ISUID and friends on an Opus file?
+		target_mode = source_stat.st_mode & 0777;
+	} else if (errno == ENOENT) {
+		target_mode = 0666 & ~get_umask();
+	} else {
+		fprintf(stderr, "warning: Could not read mode of %s: %s\n", source, strerror(errno));
+		return;
+	}
+	if (chmod(dest, target_mode) == -1)
+		fprintf(stderr, "warning: Could not set mode of %s: %s\n", dest, strerror(errno));
+}
+
+ot::status ot::partial_file::commit()
+{
+	if (file == nullptr)
+		return st::ok;
+	file.reset();
+	copy_permissions(final_name.c_str(), temporary_name.c_str());
+	if (rename(temporary_name.c_str(), final_name.c_str()) == -1)
+		return {st::standard_error,
+		        "Could not move the result file '" + temporary_name + "' to '" +
+		        final_name + "': " + strerror(errno) + "."};
+	return st::ok;
+}
+
+void ot::partial_file::abort()
+{
+	if (file == nullptr)
+		return;
+	file.reset();
+	remove(temporary_name.c_str());
+}
+
+ot::encoding_converter::encoding_converter(const char* from, const char* to)
+{
+	cd = iconv_open(to, from);
+	if (cd == (iconv_t) -1)
+		throw std::bad_alloc();
+}
+
+ot::encoding_converter::~encoding_converter()
+{
+	iconv_close(cd);
+}
+
+ot::status ot::encoding_converter::operator()(std::string_view in, std::string& out)
+{
+	iconv(cd, nullptr, nullptr, nullptr, nullptr);
+	out.clear();
+	out.reserve(in.size());
+	char* in_cursor = const_cast<char*>(in.data());
+	size_t in_left = in.size();
+	constexpr size_t chunk_size = 1024;
+	char chunk[chunk_size];
+	for (;;) {
+		char *out_cursor = chunk;
+		size_t out_left = chunk_size;
+		size_t rc = iconv(cd, &in_cursor, &in_left, &out_cursor, &out_left);
+
+		if (rc == (size_t) -1 && errno == E2BIG) {
+			// Loop normally.
+		} else if (rc == (size_t) -1) {
+			return {ot::st::badly_encoded, strerror(errno) + "."s};
+		} else if (rc != 0) {
+			return {ot::st::badly_encoded,
+				"Some characters could not be converted into the target encoding."};
+		}
+
+		out.append(chunk, out_cursor - chunk);
+		if (in_cursor == nullptr)
+			break;
+		else if (in_left == 0)
+			in_cursor = nullptr;
+	}
+	return ot::st::ok;
+}
+
+std::string ot::shell_escape(std::string_view word)
+{
+	std::string escaped_word;
+	// Pre-allocate the result, assuming most of the time enclosing it in single quotes is enough.
+	escaped_word.reserve(2 + word.size());
+
+	escaped_word += '\'';
+	for (char c : word) {
+		if (c == '\'')
+			escaped_word += "'\\''";
+		else if (c == '!')
+			escaped_word += "'\\!'";
+		else
+			escaped_word += c;
+	}
+	escaped_word += '\'';
+
+	return escaped_word;
+}
+
+ot::status ot::run_editor(std::string_view editor, std::string_view path)
+{
+	std::string command = std::string(editor) + " " + shell_escape(path);
+	int status = system(command.c_str());
+
+	if (status == -1)
+		return {st::standard_error, "waitpid error: "s + strerror(errno)};
+	else if (!WIFEXITED(status))
+		return {st::child_process_failed,
+		        "Child process did not terminate normally: "s + strerror(errno)};
+	else if (WEXITSTATUS(status) != 0)
+		return {st::child_process_failed,
+		        "Child process exited with " + std::to_string(WEXITSTATUS(status))};
+
+	return st::ok;
+}
+
+ot::status ot::get_file_timestamp(const char* path, timespec& mtime)
+{
+	struct stat st;
+	if (stat(path, &st) == -1)
+		return {st::standard_error, path + ": stat error: "s + strerror(errno)};
+#if defined(HAVE_STAT_ST_MTIM)
+	mtime = st.st_mtim;
+#elif defined(HAVE_STAT_ST_MTIMESPEC)
+	mtime = st.st_mtimespec;
+#else
+	mtime.tv_sec = st.st_mtime;
+	mtime.tv_nsec = st.st_mtimensec;
+#endif
+	return st::ok;
+}

t/CMakeLists.txt

@@ -1,16 +1,22 @@
+add_executable(system.t EXCLUDE_FROM_ALL system.cc)
+target_link_libraries(system.t ot)
+
 add_executable(opus.t EXCLUDE_FROM_ALL opus.cc)
-target_link_libraries(opus.t libopustags)
+target_link_libraries(opus.t ot)
 
 add_executable(ogg.t EXCLUDE_FROM_ALL ogg.cc)
-target_link_libraries(ogg.t libopustags)
+target_link_libraries(ogg.t ot)
 
 add_executable(cli.t EXCLUDE_FROM_ALL cli.cc)
-target_link_libraries(cli.t libopustags)
+target_link_libraries(cli.t ot)
+
+add_executable(oggdump EXCLUDE_FROM_ALL oggdump.cc)
+target_link_libraries(oggdump ot)
 
 configure_file(gobble.opus . COPYONLY)
 
 add_custom_target(
 	check
-	COMMAND prove "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
-	DEPENDS opustags gobble.opus opus.t ogg.t cli.t
+	COMMAND prove "${CMAKE_CURRENT_BINARY_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}"
+	DEPENDS opustags gobble.opus system.t opus.t ogg.t cli.t
 )

t/cli.cc

@@ -3,24 +3,195 @@
 
 #include <string.h>
 
-const char *user_comments = R"raw(
-TITLE=a b c
-
-ARTIST=X
-Artist=Y)raw";
+using namespace std::literals::string_literals;
 
 void check_read_comments()
 {
-	ot::file input = fmemopen(const_cast<char*>(user_comments), strlen(user_comments), "r");
-	auto comments = ot::read_comments(input.get());
-	auto&& expected = {"TITLE=a b c", "ARTIST=X", "Artist=Y"};
-	if (!std::equal(comments.begin(), comments.end(), expected.begin(), expected.end()))
-		throw failure("parsed user comments did not match expectations");
+	std::list<std::string> comments;
+	ot::status rc;
+	{
+		std::string txt = "TITLE=a b c\n\nARTIST=X\nArtist=Y\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		rc = ot::read_comments(input.get(), comments, false);
+		if (rc != ot::st::ok)
+			throw failure("could not read comments");
+		auto&& expected = {"TITLE=a b c", "ARTIST=X", "Artist=Y"};
+		if (!std::equal(comments.begin(), comments.end(), expected.begin(), expected.end()))
+			throw failure("parsed user comments did not match expectations");
+	}
+	{
+		std::string txt = "CORRUPTED=\xFF\xFF\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		rc = ot::read_comments(input.get(), comments, false);
+		if (rc != ot::st::badly_encoded)
+			throw failure("did not get the expected error reading corrupted data");
+	}
+	{
+		std::string txt = "RAW=\xFF\xFF\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		rc = ot::read_comments(input.get(), comments, true);
+		if (rc != ot::st::ok)
+			throw failure("could not read comments");
+		if (comments.front() != "RAW=\xFF\xFF")
+			throw failure("parsed user comments did not match expectations");
+	}
+	{
+		std::string txt = "MALFORMED\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		rc = ot::read_comments(input.get(), comments, false);
+		if (rc != ot::st::error)
+			throw failure("did not get the expected error reading malformed comments");
+	}
+}
+
+/**
+ * Wrap #ot::parse_options with a higher-level interface much more convenient for testing.
+ * In practice, the argc/argv combo are enough though for the current state of opustags.
+ */
+static ot::status parse_options(const std::vector<const char*>& args, ot::options& opt, FILE *comments)
+{
+	int argc = args.size();
+	char* argv[argc];
+	for (int i = 0; i < argc; ++i)
+		argv[i] = strdup(args[i]);
+	ot::status rc = ot::parse_options(argc, argv, opt, comments);
+	for (int i = 0; i < argc; ++i)
+		free(argv[i]);
+	return rc;
+}
+
+void check_good_arguments()
+{
+	auto parse = [](std::vector<const char*> args) {
+		ot::options opt;
+		std::string txt = "N=1\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		ot::status rc = parse_options(args, opt, input.get());
+		if (rc.code != ot::st::ok)
+			throw failure("unexpected option parsing error");
+		return opt;
+	};
+
+	ot::options opt;
+	opt = parse({"opustags", "--help", "x", "-o", "y"});
+	if (!opt.print_help)
+		throw failure("did not catch --help");
+
+	opt = parse({"opustags", "x", "--output", "y", "-D", "-s", "X=Y Z", "-d", "a=b"});
+	if (opt.paths_in.size() != 1 || opt.paths_in.front() != "x" || !opt.path_out ||
+	    opt.path_out != "y" || !opt.delete_all || opt.overwrite || opt.to_delete.size() != 2 ||
+	    opt.to_delete.front() != "X" || *std::next(opt.to_delete.begin()) != "a=b" ||
+	    opt.to_add != std::list<std::string>{"X=Y Z"})
+		throw failure("unexpected option parsing result for case #1");
+
+	opt = parse({"opustags", "-S", "x", "-S", "-a", "x=y z", "-i"});
+	if (opt.paths_in.size() != 1 || opt.paths_in.front() != "x" || opt.path_out ||
+	    !opt.overwrite || opt.to_delete.size() != 0 ||
+	    opt.to_add != std::list<std::string>{"N=1", "x=y z"})
+		throw failure("unexpected option parsing result for case #2");
+
+	opt = parse({"opustags", "-i", "x", "y", "z"});
+	if (opt.paths_in.size() != 3 || opt.paths_in[0] != "x" || opt.paths_in[1] != "y" ||
+	    opt.paths_in[2] != "z" || !opt.overwrite || !opt.in_place)
+		throw failure("unexpected option parsing result for case #3");
+
+	opt = parse({"opustags", "-ie", "x"});
+	if (opt.paths_in.size() != 1 || opt.paths_in[0] != "x" ||
+	    !opt.edit_interactively || !opt.overwrite || !opt.in_place)
+		throw failure("unexpected option parsing result for case #4");
+
+	opt = parse({"opustags", "-a", "X=\xFF", "--raw", "x"});
+	if (!opt.raw || opt.to_add.front() != "X=\xFF")
+		throw failure("--raw did not disable transcoding");
+}
+
+void check_bad_arguments()
+{
+	auto error_code_case = [](std::vector<const char*> args, const char* message, ot::st error_code, const std::string& name) {
+		ot::options opt;
+		std::string txt = "N=1\nINVALID"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		ot::status rc = parse_options(args, opt, input.get());
+		if (rc.code != error_code)
+			throw failure("bad error code for case " + name);
+		if (rc.message != message)
+			throw failure("bad error message for case " + name + ", got: " + rc.message);
+	};
+	auto error_case = [&error_code_case](std::vector<const char*> args, const char* message, const std::string& name) {
+		 error_code_case(args, message, ot::st::bad_arguments, name);
+	};
+	error_case({"opustags"}, "No arguments specified. Use -h for help.", "no arguments");
+	error_case({"opustags", "-a", "X"}, "Comment does not contain an equal sign: X.", "bad comment for -a");
+	error_case({"opustags", "--set", "X"}, "Comment does not contain an equal sign: X.", "bad comment for --set");
+	error_case({"opustags", "-a"}, "Missing value for option '-a'.", "short option with missing value");
+	error_case({"opustags", "--add"}, "Missing value for option '--add'.", "long option with missing value");
+	error_case({"opustags", "-x"}, "Unrecognized option '-x'.", "unrecognized short option");
+	error_case({"opustags", "--derp"}, "Unrecognized option '--derp'.", "unrecognized long option");
+	error_case({"opustags", "-x=y"}, "Unrecognized option '-x'.", "unrecognized short option with value");
+	error_case({"opustags", "--derp=y"}, "Unrecognized option '--derp=y'.", "unrecognized long option with value");
+	error_case({"opustags", "-aX=Y"}, "Exactly one input file must be specified.", "no input file");
+	error_case({"opustags", "-i", "-o", "/dev/null", "-"}, "Cannot combine --in-place and --output.", "in-place + output");
+	error_case({"opustags", "-S", "-"}, "Cannot use standard input as input file when --set-all is specified.",
+	                                    "set all and read opus from stdin");
+	error_case({"opustags", "-i", "-"}, "Cannot modify standard input in place.", "write stdin in-place");
+	error_case({"opustags", "-o", "x", "--output", "y", "z"},
+	           "Cannot specify --output more than once.", "double output");
+	error_code_case({"opustags", "-S", "x"}, "Malformed tag: INVALID", ot::st::error, "attempt to read invalid argument with -S");
+	error_case({"opustags", "-o", "", "--output", "y", "z"},
+	           "Cannot specify --output more than once.", "double output with first filename empty");
+	error_case({"opustags", "-e", "-i", "x", "y"},
+	           "Exactly one input file must be specified.", "editing interactively two files at once");
+	error_case({"opustags", "--edit", "-", "-o", "x"},
+	           "Cannot edit interactively when standard input or standard output are already used.",
+	           "editing interactively from stdandard intput");
+	error_case({"opustags", "--edit", "x", "-o", "-"},
+	           "Cannot edit interactively when standard input or standard output are already used.",
+	           "editing interactively to stdandard output");
+	error_case({"opustags", "--edit", "x"}, "Cannot edit interactively when no output is specified.", "editing without output");
+	error_case({"opustags", "--edit", "x", "-i", "-a", "X=Y"}, "Cannot mix --edit with -adDsS.", "mixing -e and -a");
+	error_case({"opustags", "--edit", "x", "-i", "-d", "X"}, "Cannot mix --edit with -adDsS.", "mixing -e and -d");
+	error_case({"opustags", "--edit", "x", "-i", "-D"}, "Cannot mix --edit with -adDsS.", "mixing -e and -D");
+	error_case({"opustags", "--edit", "x", "-i", "-S"}, "Cannot mix --edit with -adDsS.", "mixing -e and -S");
+	error_case({"opustags", "-d", "\xFF", "x"},
+	           "Could not encode argument into UTF-8: Invalid or incomplete multibyte or wide character.",
+	           "-d with binary data");
+	error_case({"opustags", "-a", "X=\xFF", "x"},
+	           "Could not encode argument into UTF-8: Invalid or incomplete multibyte or wide character.",
+	           "-a with binary data");
+	error_case({"opustags", "-s", "X=\xFF", "x"},
+	           "Could not encode argument into UTF-8: Invalid or incomplete multibyte or wide character.",
+	           "-s with binary data");
+}
+
+static void check_delete_comments()
+{
+	using C = std::list<std::string>;
+	C original = {"TITLE=X", "Title=Y", "Title=Z", "ARTIST=A", "artIst=B"};
+
+	C edited = original;
+	ot::delete_comments(edited, "derp");
+	if (!std::equal(edited.begin(), edited.end(), original.begin(), original.end()))
+		throw failure("should not have deleted anything");
+
+	ot::delete_comments(edited, "Title");
+	C expected = {"ARTIST=A", "artIst=B"};
+	if (!std::equal(edited.begin(), edited.end(), expected.begin(), expected.end()))
+		throw failure("did not delete all titles correctly");
+
+	edited = original;
+	ot::delete_comments(edited, "titlE=Y");
+	ot::delete_comments(edited, "Title=z");
+	expected = {"TITLE=X", "Title=Z", "ARTIST=A", "artIst=B"};
+	if (!std::equal(edited.begin(), edited.end(), expected.begin(), expected.end()))
+		throw failure("did not delete a specific title correctly");
 }
 
 int main(int argc, char **argv)
 {
-	std::cout << "1..1\n";
+	std::cout << "1..4\n";
 	run(check_read_comments, "check tags parsing");
+	run(check_good_arguments, "check options parsing");
+	run(check_bad_arguments, "check options parsing errors");
+	run(check_delete_comments, "delete comments");
 	return 0;
 }

t/ogg.cc

@@ -11,36 +11,36 @@ static void check_ref_ogg()
 
 	ot::ogg_reader reader(input.get());
 
-	ot::status rc = reader.read_page();
+	ot::status rc = reader.next_page();
 	if (rc != ot::st::ok)
 		throw failure("could not read the first page");
-	rc = reader.read_packet();
+	if (!ot::is_opus_stream(reader.page))
+		throw failure("failed to identify the stream as opus");
+	rc = reader.process_header_packet([](ogg_packet& p) {
+		if (p.bytes != 19)
+			throw failure("unexpected length for the first packet");
+		return ot::st::ok;
+	});
 	if (rc != ot::st::ok)
 		throw failure("could not read the first packet");
-	if (reader.packet.bytes != 19)
-		throw failure("unexpected length for the first packet");
-	rc = reader.read_packet();
-	if (rc != ot::st::end_of_page)
-		throw failure("got an unexpected second packet on the first page");
 
-	rc = reader.read_page();
+	rc = reader.next_page();
 	if (rc != ot::st::ok)
 		throw failure("could not read the second page");
-	rc = reader.read_packet();
+	rc = reader.process_header_packet([](ogg_packet& p) {
+		if (p.bytes != 62)
+			throw failure("unexpected length for the second packet");
+		return ot::st::ok;
+	});
 	if (rc != ot::st::ok)
 		throw failure("could not read the second packet");
-	if (reader.packet.bytes != 62)
-		throw failure("unexpected length for the first packet");
-	rc = reader.read_packet();
-	if (rc != ot::st::end_of_page)
-		throw failure("got an unexpected second packet on the second page");
 
 	while (!ogg_page_eos(&reader.page)) {
-		rc = reader.read_page();
+		rc = reader.next_page();
 		if (rc != ot::st::ok)
 			throw failure("failure reading a page");
 	}
-	rc = reader.read_page();
+	rc = reader.next_page();
 	if (rc != ot::st::end_of_stream)
 		throw failure("did not correctly detect the end of stream");
 }
@@ -63,9 +63,8 @@ static bool same_packet(const ogg_packet& lhs, const ogg_packet& rhs)
  */
 static void check_memory_ogg()
 {
-	const ogg_packet first_packet  = make_packet("First");
-	const ogg_packet second_packet = make_packet("Second");
-	const ogg_packet third_packet  = make_packet("Third");
+	ogg_packet first_packet  = make_packet("First");
+	ogg_packet second_packet = make_packet("Second");
 	std::vector<unsigned char> my_ogg(128);
 	size_t my_ogg_size;
 	ot::status rc;
@@ -75,29 +74,14 @@ static void check_memory_ogg()
 		if (output == nullptr)
 			throw failure("could not open the output stream");
 		ot::ogg_writer writer(output.get());
-		rc = writer.prepare_stream(1234);
-		if (rc != ot::st::ok)
-			throw failure("could not prepare the stream for the first page");
-		writer.write_packet(first_packet);
+		writer.write_header_packet(1234, 0, first_packet);
 		if (rc != ot::st::ok)
 			throw failure("could not write the first packet");
-		writer.flush_page();
-		if (rc != ot::st::ok)
-			throw failure("could not flush the first page");
-		writer.prepare_stream(1234);
-		if (rc != ot::st::ok)
-			throw failure("could not prepare the stream for the second page");
-		writer.write_packet(second_packet);
+		writer.write_header_packet(1234, 1, second_packet);
 		if (rc != ot::st::ok)
 			throw failure("could not write the second packet");
-		writer.write_packet(third_packet);
-		if (rc != ot::st::ok)
-			throw failure("could not write the third packet");
-		writer.flush_page();
-		if (rc != ot::st::ok)
-			throw failure("could not flush the second page");
 		my_ogg_size = ftell(output.get());
-		if (my_ogg_size != 73)
+		if (my_ogg_size != 67)
 			throw failure("unexpected output size");
 	}
 
@@ -106,43 +90,81 @@ static void check_memory_ogg()
 		if (input == nullptr)
 			throw failure("could not open the input stream");
 		ot::ogg_reader reader(input.get());
-		rc = reader.read_page();
+		rc = reader.next_page();
 		if (rc != ot::st::ok)
 			throw failure("could not read the first page");
-		rc = reader.read_packet();
+		rc = reader.process_header_packet([&first_packet](ogg_packet &p) {
+			if (!same_packet(p, first_packet))
+				throw failure("unexpected content in the first packet");
+			return ot::st::ok;
+		});
 		if (rc != ot::st::ok)
 			throw failure("could not read the first packet");
-		if (!same_packet(reader.packet, first_packet))
-			throw failure("unexpected content in the first packet");
-		rc = reader.read_packet();
-		if (rc != ot::st::end_of_page)
-			throw failure("unexpected second packet in the first page");
-		rc = reader.read_page();
+		rc = reader.next_page();
 		if (rc != ot::st::ok)
 			throw failure("could not read the second page");
-		rc = reader.read_packet();
+		rc = reader.process_header_packet([&second_packet](ogg_packet &p) {
+			if (!same_packet(p, second_packet))
+				throw failure("unexpected content in the second packet");
+			return ot::st::ok;
+		});
 		if (rc != ot::st::ok)
 			throw failure("could not read the second packet");
-		if (!same_packet(reader.packet, second_packet))
-			throw failure("unexpected content in the second packet");
-		rc = reader.read_packet();
-		if (rc != ot::st::ok)
-			throw failure("could not read the third packet");
-		if (!same_packet(reader.packet, third_packet))
-			throw failure("unexpected content in the third packet");
-		rc = reader.read_packet();
-		if (rc != ot::st::end_of_page)
-			throw failure("unexpected third packet in the second page");
-		rc = reader.read_page();
+		rc = reader.next_page();
 		if (rc != ot::st::end_of_stream)
 			throw failure("unexpected third page");
 	}
 }
 
+void check_bad_stream()
+{
+	auto err_msg = "did not detect the stream is not an ogg stream";
+	ot::file input = fmemopen((void*) err_msg, 20, "r");
+	ot::ogg_reader reader(input.get());
+	ot::status rc = reader.next_page();
+	if (rc != ot::st::bad_stream)
+		throw failure(err_msg);
+}
+
+void check_identification()
+{
+	auto good_header = (unsigned char*)
+		"\x4f\x67\x67\x53\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x42\xf2"
+		"\xe6\xc7\x00\x00\x00\x00\x7e\xc3\x57\x2b\x01\x13";
+	auto good_body = (unsigned char*) "OpusHeadABCD";
+
+	ogg_page id;
+	id.header = good_header;
+	id.header_len = 28;
+	id.body = good_body;
+	id.body_len = 12;
+	if (!ot::is_opus_stream(id))
+		throw failure("could not identify opus header");
+
+	// Bad body
+	id.body_len = 7;
+	if (ot::is_opus_stream(id))
+		throw failure("opus header was too short to be valid");
+	id.body_len = 12;
+	id.body = (unsigned char*) "Not_OpusHead";
+	if (ot::is_opus_stream(id))
+		throw failure("was not an opus header");
+	id.body = good_body;
+
+	// Remove the BoS bit from the header.
+	id.header = (unsigned char*)
+		"\x4f\x67\x67\x53\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x42\xf2"
+		"\xe6\xc7\x00\x00\x00\x00\x7e\xc3\x57\x2b\x01\x13";
+	if (ot::is_opus_stream(id))
+		throw failure("was not the beginning of a stream");
+}
+
 int main(int argc, char **argv)
 {
-	std::cout << "1..2\n";
+	std::cout << "1..4\n";
 	run(check_ref_ogg, "check a reference ogg stream");
 	run(check_memory_ogg, "build and check a fresh stream");
+	run(check_bad_stream, "read a non-ogg stream");
+	run(check_identification, "stream identification");
 	return 0;
 }

t/oggdump.cc

@@ -0,0 +1,42 @@
+/**
+ * \file t/oggdump.cc
+ *
+ * Dump brief information about the pages containted in an Ogg file.
+ *
+ * This tool is not build by default or installed, and is mainly meant to help understand how Ogg
+ * files are built, and to debug.
+ */
+
+#include <opustags.h>
+
+#include <iostream>
+#include <string.h>
+
+int main(int argc, char** argv)
+{
+	if (argc != 2) {
+		std::cerr << "Usage: oggdump FILE\n";
+		return 1;
+	}
+	ot::file input = fopen(argv[1], "r");
+	if (input == nullptr) {
+		std::cerr << "Error opening '" << argv[1] << "': " << strerror(errno) << "\n";
+		return 1;
+	}
+	ot::ogg_reader reader(input.get());
+	ot::status rc;
+	while ((rc = reader.read_page()) == ot::st::ok) {
+		std::cout << "Stream " << ogg_page_serialno(&reader.page) << ", "
+		             "page #" << ogg_page_pageno(&reader.page) << ", "
+		          << ogg_page_packets(&reader.page) << " packet(s)";
+		if (ogg_page_bos(&reader.page)) std::cout << ", BoS";
+		if (ogg_page_eos(&reader.page)) std::cout << ", EoS";
+		if (ogg_page_continued(&reader.page)) std::cout << ", continued";
+		std::cout << "\n";
+	}
+	if (rc != ot::st::ok && rc != ot::st::end_of_stream) {
+		std::cerr << "error: " << rc.message << "\n";
+		return 1;
+	}
+	return 0;
+}

t/opus.cc

@@ -5,24 +5,6 @@
 
 using namespace std::literals::string_literals;
 
-static void check_identification()
-{
-	ogg_packet packet {};
-	packet.packet = (unsigned char*) "OpusHead..";
-	packet.bytes = 10;
-	if (ot::validate_identification_header(packet) != ot::st::ok)
-		throw failure("did not accept a good OpusHead");
-
-	packet.bytes = 7;
-	if (ot::validate_identification_header(packet) != ot::st::cut_magic_number)
-		throw failure("accepted an OpusHead that is too short");
-
-	packet.packet = (unsigned char*) "NotOpusHead";
-	packet.bytes = 11;
-	if (ot::validate_identification_header(packet) != ot::st::bad_magic_number)
-		throw failure("did not report the right status for a bad OpusHead");
-}
-
 static const char standard_OpusTags[] =
 	"OpusTags"
 	"\x14\x00\x00\x00" "opustags test packet"
@@ -156,8 +138,7 @@ static void recode_padding()
 
 int main()
 {
-	std::cout << "1..5\n";
-	run(check_identification, "check the OpusHead packet");
+	std::cout << "1..4\n";
 	run(parse_standard, "parse a standard OpusTags packet");
 	run(parse_corrupted, "correctly reject invalid packets");
 	run(recode_standard, "recode a standard OpusTags packet");

t/opustags.t

@@ -4,16 +4,24 @@ use strict;
 use warnings;
 use utf8;
 
-use Test::More tests => 27;
+use Test::More tests => 50;
 
 use Digest::MD5;
 use File::Basename;
+use File::Copy;
 use IPC::Open3;
+use List::MoreUtils qw(any);
 use Symbol 'gensym';
 
 my $opustags = '../opustags';
 BAIL_OUT("$opustags does not exist or is not executable") if (! -x $opustags);
 
+my $is_utf8;
+open(my $ctype, 'locale -k LC_CTYPE |');
+while (<$ctype>) { $is_utf8 = 1 if (/^charmap="UTF-?8"$/i) }
+close($ctype);
+BAIL_OUT("this test must be run from an UTF-8 environment") unless $is_utf8;
+
 sub opustags {
 	my %opt;
 	%opt = %{pop @_} if ref $_[-1];
@@ -36,44 +44,48 @@ sub opustags {
 # Tests related to the overall opustags executable, like the help message.
 # No Opus file is manipulated here.
 
-my $usage = opustags();
-$usage->[0] =~ /^([^\n]*+)/;
+is_deeply(opustags(), ['', <<EOF, 256], 'no options is a failure');
+error: No arguments specified. Use -h for help.
+EOF
+
+my $help = opustags('--help');
+$help->[0] =~ /^([^\n]*+)/;
 my $version = $1;
 like($version, qr/^opustags version (\d+\.\d+\.\d+)/, 'get the version string');
 
-is_deeply($usage, [<<"EOF", "", 0], 'no options show the usage');
-$version
-Usage: opustags --help
-       opustags [OPTIONS] FILE
-       opustags OPTIONS FILE -o FILE
-EOF
-
-my $help = <<"EOF";
+my $expected_help = <<"EOF";
 $version
 
 Usage: opustags --help
        opustags [OPTIONS] FILE
+       opustags OPTIONS -i FILE...
        opustags OPTIONS FILE -o FILE
 
 Options:
-  -h, --help              print this help
-  -o, --output FILE       set the output file
-  -i, --in-place          overwrite the input file instead of writing a different output file
-  -y, --overwrite         overwrite the output file if it already exists
-  -a, --add FIELD=VALUE   add a comment
-  -d, --delete FIELD      delete all previously existing comments of a specific type
-  -D, --delete-all        delete all the previously existing comments
-  -s, --set FIELD=VALUE   replace a comment (shorthand for --delete FIELD --add FIELD=VALUE)
-  -S, --set-all           replace all the comments with the ones read from standard input
+  -h, --help                    print this help
+  -o, --output FILE             specify the output file
+  -i, --in-place                overwrite the input files
+  -y, --overwrite               overwrite the output file if it already exists
+  -a, --add FIELD=VALUE         add a comment
+  -d, --delete FIELD[=VALUE]    delete previously existing comments
+  -D, --delete-all              delete all the previously existing comments
+  -s, --set FIELD=VALUE         replace a comment
+  -S, --set-all                 import comments from standard input
+  -e, --edit                    edit tags interactively in VISUAL/EDITOR
+  --raw                         disable encoding conversion
 
 See the man page for extensive documentation.
 EOF
 
-is_deeply(opustags('--help'), [$help, '', 0], '--help displays the help message');
-is_deeply(opustags('-h'), [$help, '', 0], '-h displays the help message too');
+is_deeply(opustags('--help'), [$expected_help, '', 0], '--help displays the help message');
+is_deeply(opustags('-h'), [$expected_help, '', 0], '-h displays the help message too');
 
 is_deeply(opustags('--derp'), ['', <<"EOF", 256], 'unrecognized option shows an error');
-$opustags: unrecognized option '--derp'
+error: Unrecognized option '--derp'.
+EOF
+
+is_deeply(opustags('../opustags'), ['', <<"EOF", 256], 'not an Ogg stream');
+../opustags: error: Input is not a valid Ogg file.
 EOF
 
 ####################################################################################################
@@ -93,27 +105,32 @@ encoder=Lavc58.18.100 libopus
 EOF
 
 unlink('out.opus');
+my $previous_umask = umask(0022);
 is_deeply(opustags(qw(gobble.opus -o out.opus)), ['', '', 0], 'copy the file without changes');
 is(md5('out.opus'), '111a483596ac32352fbce4d14d16abd2', 'the copy is faithful');
+is((stat 'out.opus')[2] & 0777, 0644, 'apply umask on new files');
+umask($previous_umask);
 
 # empty out.opus
 { my $fh; open($fh, '>', 'out.opus') and close($fh) or die }
 is_deeply(opustags(qw(gobble.opus -o out.opus)), ['', <<'EOF', 256], 'refuse to override');
-error: 'out.opus' already exists (use -y to overwrite)
+gobble.opus: error: 'out.opus' already exists. Use -y to overwrite.
 EOF
 is(md5('out.opus'), 'd41d8cd98f00b204e9800998ecf8427e', 'the output wasn\'t written');
 
-is_deeply(opustags(qw(out.opus -o out.opus)), ['', <<'EOF', 256], 'output and input can\'t be the same');
-error: Input and output files are the same
-EOF
+is_deeply(opustags(qw(gobble.opus -o /dev/null)), ['', '', 0], 'write to /dev/null');
 
+chmod(0604, 'out.opus');
 is_deeply(opustags(qw(gobble.opus -o out.opus --overwrite)), ['', '', 0], 'overwrite');
 is(md5('out.opus'), '111a483596ac32352fbce4d14d16abd2', 'successfully overwritten');
+is((stat 'out.opus')[2] & 0777, 0604, 'overwriting preserves output file\'s mode');
 
+chmod(0700, 'out.opus');
 is_deeply(opustags(qw(--in-place out.opus -a A=B --add=A=C --add), "TITLE=Foo Bar",
                    qw(--delete A --add TITLE=七面鳥 --set encoder=whatever -s 1=2 -s X=1 -a X=2 -s X=3)),
           ['', '', 0], 'complex tag editing');
 is(md5('out.opus'), '66780307a6081523dc9040f3c47b0448', 'check the footprint');
+is((stat 'out.opus')[2] & 0777, 0700, 'in-place editing preserves file mode');
 
 is_deeply(opustags('out.opus'), [<<'EOF', '', 0], 'check the tags written');
 A=B
@@ -127,7 +144,8 @@ X=2
 X=3
 EOF
 
-is_deeply(opustags(qw(out.opus -d A -d foo -s X=4 -a TITLE=gobble -d TITLE)), [<<'EOF', '', 0], 'dry editing');
+is_deeply(opustags(qw(out.opus -d A -d foo -s X=4 -a TITLE=gobble -d title=七面鳥)), [<<'EOF', '', 0], 'dry editing');
+TITLE=Foo Bar
 encoder=whatever
 1=2
 X=4
@@ -136,12 +154,20 @@ EOF
 is(md5('out.opus'), '66780307a6081523dc9040f3c47b0448', 'the file did not change');
 
 is_deeply(opustags(qw(-i out.opus -a fatal=yes -a FOO -a BAR)), ['', <<'EOF', 256], 'bad tag with --add');
-invalid comment: 'FOO'
+error: Comment does not contain an equal sign: FOO.
 EOF
 is(md5('out.opus'), '66780307a6081523dc9040f3c47b0448', 'the file did not change');
 
+is_deeply(opustags('out.opus', '-D', '-a', "X=foo\nbar\tquux"), [<<'END_OUT', <<'END_ERR', 0], 'control characters');
+X=foo
+bar	quux
+END_OUT
+warning: Some tags contain unsupported newline characters.
+warning: Some tags contain control characters.
+END_ERR
+
 is_deeply(opustags(qw(-i out.opus -s fatal=yes -s FOO -s BAR)), ['', <<'EOF', 256], 'bad tag with --set');
-invalid comment: 'FOO'
+error: Comment does not contain an equal sign: FOO.
 EOF
 is(md5('out.opus'), '66780307a6081523dc9040f3c47b0448', 'the file did not change');
 
@@ -155,6 +181,7 @@ ARTIST=七面鳥
 
 A=A
 X=Y
+#IGNORE=COMMENTS
 END_IN
 OK=yes again
 ARTIST=七面鳥
@@ -164,18 +191,12 @@ A=B
 X=Z
 END_OUT
 
-is_deeply(opustags(qw(out.opus -S), {in => <<'END_IN'}), [<<'END_OUT', <<'END_ERR', 0], 'set all with bad tags');
+is_deeply(opustags(qw(out.opus -S), {in => <<'END_IN'}), [<<'END_OUT', <<'END_ERR', 256], 'set all with bad tags');
 whatever
-
-# thing
-!
 wrong=yes
 END_IN
-wrong=yes
 END_OUT
-warning: skipping malformed tag
-warning: skipping malformed tag
-warning: skipping malformed tag
+error: Malformed tag: whatever
 END_ERR
 
 sub slurp {
@@ -191,3 +212,102 @@ my $data = slurp 'out.opus';
 is_deeply(opustags('-', '-o', '-', {in => $data, mode => ':raw'}), [$data, '', 0], 'read opus from stdin and write to stdout');
 
 unlink('out.opus');
+
+# Test --in-place
+unlink('out2.opus');
+copy('gobble.opus', 'out.opus');
+is_deeply(opustags(qw(out.opus --add BAR=baz -o out2.opus)), ['', '', 0], 'process multiple files with --in-place');
+is_deeply(opustags(qw(--in-place --add FOO=bar out.opus out2.opus)), ['', '', 0], 'process multiple files with --in-place');
+is(md5('out.opus'), '30ba30c4f236c09429473f36f8f861d2', 'the tags were added correctly (out.opus)');
+is(md5('out2.opus'), '0a4d20c287b2e46b26cb0eee353c2069', 'the tags were added correctly (out2.opus)');
+
+unlink('out.opus');
+unlink('out2.opus');
+
+####################################################################################################
+# Interactive edition
+
+$ENV{EDITOR} = 'sed -i -e y/aeiou/AEIOU/ `sleep 0.1`';
+is_deeply(opustags('gobble.opus', '-eo', "'screaming !'.opus"), ['', '', 0], 'edit a file with EDITOR');
+is(md5("'screaming !'.opus"), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were modified');
+
+$ENV{EDITOR} = 'true';
+is_deeply(opustags('-ie', "'screaming !'.opus"), ['', "Cancelling edition because the tags file was not modified.\n", 256], 'close -e without saving');
+is(md5("'screaming !'.opus"), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were not modified');
+
+$ENV{EDITOR} = 'false';
+is_deeply(opustags('-ie', "'screaming !'.opus"), ['', "'screaming !'.opus: error: Child process exited with 1\n", 256], 'editor exiting with an error');
+is(md5("'screaming !'.opus"), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were not modified');
+
+unlink("'screaming !'.opus");
+
+####################################################################################################
+# Test muxed streams
+
+system('ffmpeg -loglevel error -y -i gobble.opus -c copy -map 0:0 -map 0:0 -shortest muxed.ogg') == 0
+	or BAIL_OUT('could not create a muxed stream');
+
+is_deeply(opustags('muxed.ogg'), ['', <<'END_ERR', 256], 'muxed streams detection');
+muxed.ogg: error: Muxed streams are not supported yet.
+END_ERR
+
+unlink('muxed.ogg');
+
+####################################################################################################
+# Locale
+
+my $locale = 'en_US.iso88591';
+my @all_locales = split(' ', `locale -a`);
+
+SKIP: {
+skip "locale $locale is not present", 5 unless (any { $_ eq $locale } @all_locales);
+
+opustags(qw(gobble.opus -a TITLE=七面鳥 -a ARTIST=éàç -o out.opus -y));
+
+local $ENV{LC_ALL} = $locale;
+local $ENV{LANGUAGE} = '';
+
+is_deeply(opustags(qw(-S out.opus), {in => <<"END_IN", mode => ':raw'}), [<<"END_OUT", '', 0], 'set all in ISO-8859-1');
+T=\xef\xef\xf6
+END_IN
+T=\xef\xef\xf6
+END_OUT
+
+is_deeply(opustags('-i', 'out.opus', "--add=I=\xf9\xce", {mode => ':raw'}), ['', '', 0], 'write tags in ISO-8859-1');
+
+is_deeply(opustags('out.opus', {mode => ':raw'}), [<<"END_OUT", <<"END_ERR", 256], 'read tags in ISO-8859-1 with incompatible characters');
+encoder=Lavc58.18.100 libopus
+END_OUT
+out.opus: error: Invalid or incomplete multibyte or wide character. See --raw.
+END_ERR
+
+is_deeply(opustags(qw(out.opus -d TITLE -d ARTIST), {mode => ':raw'}), [<<"END_OUT", '', 0], 'read tags in ISO-8859-1');
+encoder=Lavc58.18.100 libopus
+I=\xf9\xce
+END_OUT
+
+$ENV{LC_ALL} = '';
+
+is_deeply(opustags('out.opus'), [<<"END_OUT", '', 0], 'read tags in UTF-8');
+encoder=Lavc58.18.100 libopus
+TITLE=七面鳥
+ARTIST=éàç
+I=ùÎ
+END_OUT
+
+unlink('out.opus');
+}
+
+####################################################################################################
+# Raw edition
+
+is_deeply(opustags(qw(-S gobble.opus -o out.opus --raw -a), "U=\xFE", {in => <<"END_IN", mode => ':raw'}), ['', '', 0], 'raw set-all with binary data');
+T=\xFF
+END_IN
+
+is_deeply(opustags(qw(out.opus --raw), { mode => ':raw' }), [<<"END_OUT", '', 0], 'raw read');
+T=\xFF
+U=\xFE
+END_OUT
+
+unlink('out.opus');

t/system.cc

@@ -0,0 +1,67 @@
+#include <opustags.h>
+#include "tap.h"
+
+#include <string.h>
+#include <unistd.h>
+
+void check_partial_files()
+{
+	static const char* result = "partial_file.test";
+	std::string name;
+	{
+		ot::partial_file bad_tmp;
+		is(bad_tmp.open("/dev/null"), ot::st::standard_error,
+		   "opening a device as a partial file fails");
+		is(bad_tmp.open(result), ot::st::ok,
+		   "opening a regular partial file works");
+		name = bad_tmp.name();
+		if (name.size() != strlen(result) + 12 ||
+		    name.compare(0, strlen(result), result) != 0)
+			throw failure("the temporary name is surprising: " + name);
+	}
+	is(access(name.c_str(), F_OK), -1, "expect the temporary file is deleted");
+
+	ot::partial_file good_tmp;
+	is(good_tmp.open(result), ot::st::ok, "open the partial file");
+	name = good_tmp.name();
+	is(good_tmp.commit(), ot::st::ok, "commit the result file");
+	is(access(name.c_str(), F_OK), -1, "expect the temporary file is deleted");
+	is(access(result, F_OK), 0, "expect the final result file");
+	is(remove(result), 0, "remove the result file");
+}
+
+void check_converter()
+{
+	const char* ephemere_iso = "\xc9\x70\x68\xe9\x6d\xe8\x72\x65";
+	ot::encoding_converter to_utf8("ISO_8859-1", "UTF-8");
+	ot::encoding_converter from_utf8("UTF-8", "ISO_8859-1//IGNORE");
+	std::string out;
+
+	ot::status rc = to_utf8(ephemere_iso, out);
+	is(rc, ot::st::ok, "conversion to UTF-8 is successful");
+	is(out, "Éphémère", "conversion to UTF-8 is correct");
+
+	rc = from_utf8("Éphémère", out);
+	is(rc, ot::st::ok, "conversion from UTF-8 is successful");
+	is(out, ephemere_iso, "conversion from UTF-8 is correct");
+
+	rc = from_utf8("\xFF\xFF", out);
+	is(rc, ot::st::badly_encoded, "conversion from bad UTF-8 fails");
+}
+
+void check_shell_esape()
+{
+	is(ot::shell_escape("foo"), "'foo'", "simple string");
+	is(ot::shell_escape("a'b"), "'a'\\''b'", "string with a simple quote");
+	is(ot::shell_escape("a!b"), "'a'\\!'b'", "string with a bang");
+	is(ot::shell_escape("a!b'c!d'e"), "'a'\\!'b'\\''c'\\!'d'\\''e'", "string with a bang");
+}
+
+int main(int argc, char **argv)
+{
+	plan(3);
+	run(check_partial_files, "test partial files");
+	run(check_converter, "test encoding converter");
+	run(check_shell_esape, "test shell escaping");
+	return 0;
+}

t/tap.h

@@ -3,6 +3,11 @@
  *
  * \brief
  * Helpers for following the Test Anything Protocol.
+ *
+ * Its interface mimics Test::More from Perl:
+ * https://perldoc.perl.org/Test/More.html
+ *
+ * Unlike Test::More, a test failure raises an exception and aborts the whole subtest.
  */
 
 #pragma once
@@ -10,9 +15,10 @@
 #include <exception>
 #include <iostream>
 
-class failure : public std::runtime_error {
-public:
-	failure(const char *message) : std::runtime_error(message) {}
+inline namespace tap {
+
+struct failure : std::runtime_error {
+	failure(const std::string& what) : std::runtime_error(what) {}
 };
 
 template <typename F>
@@ -23,7 +29,36 @@ static void run(F test, const char *name)
 		test();
 		ok = true;
 	} catch (failure& e) {
-		std::cout << "# " << e.what() << "\n";
+		std::cerr << "# fail: " << e.what() << "\n";
 	}
 	std::cout << (ok ? "ok" : "not ok") << " - " << name << "\n";
 }
+
+void plan(int tests)
+{
+	std::cout << "1.." << tests << "\n";
+}
+
+template <typename T, typename U>
+void is(const T& got, const U& expected, const char* name)
+{
+	if (got != expected) {
+		std::cerr << "# got: " << got << "\n"
+		             "# expected: " << expected << "\n";
+		throw failure(name);
+	}
+}
+
+template <>
+void is(const ot::status& got, const ot::st& expected, const char* name)
+{
+	if (got.code != expected) {
+		if (got.code == ot::st::ok)
+			std::cerr << "# unexpected success\n";
+		else
+			std::cerr << "# unexpected error: " << got.message << "\n";
+		throw failure(name);
+	}
+}
+
+}