Mention METADATA_BLOCK_PICTURE in the man page

* fixes fmang/opustags#69

.editorconfig

@@ -0,0 +1,8 @@
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+charset = utf-8
+ident_style = tab
+max_line_length = 100

.github/workflows/ci.yaml

@@ -0,0 +1,30 @@
+name: Continuous Integration
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+  workflow_dispatch:
+env:
+  LC_CTYPE: C.UTF-8
+  CMAKE_COLOR_DIAGNOSTICS: ON
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout git repository
+      uses: actions/checkout@v4
+    - name: Install dependencies
+      run: |
+        sudo apt install cmake g++ pkg-config libogg-dev ffmpeg libtest-harness-perl libtest-deep-perl liblist-moreutils-perl libtest-utf8-perl
+    - name: Build
+      env:
+        CXX: g++
+        CXXFLAGS: -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_DEBUG -O2 -flto=auto -g -Wall -Wextra -Werror=format-security -fstack-protector-strong -fstack-clash-protection -fcf-protection -fsanitize=address,undefined
+        LDFLAGS: -fsanitize=address,undefined
+      run: |
+        cmake -B target -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON
+        cmake --build target
+    - name: Test
+      run: |
+        cmake --build target --target check

CHANGELOG.md

@@ -1,6 +1,69 @@
 opustags changelog
 ==================
 
+1.10.1 - 2024-05-19
+-------------------
+
+Fix a build error on recent systems.
+
+1.10.0 - 2024-05-03
+-------------------
+
+- Introduce -z to delimit tags with null bytes.
+
+This option makes it possible to leverage GNU sed or GNU grep for automated tag edition with
+`opustags -z … | sed -z … | opustags -z -S …`, while also supporting multi-line tags.
+
+1.9.0 - 2023-06-07
+------------------
+
+- Introduce --vendor and --set-vendor.
+- Close the input file before finalizing the output, in order to fix --in-place on SMB drives.
+
+1.8.0 - 2023-03-07
+------------------
+
+- Introduce --set-cover and --output-cover.
+
+opustags is now able to extract and edit the cover art of Opus files. The underlying
+METADATA_BLOCK_PICTURE tag will still appear as a regular tag, but you won’t have to handle it
+manually anymore.
+
+1.7.0 - 2023-02-13
+------------------
+
+- Support arbitrary large OpusTags headers.
+- Handle multiline tags by prefixing their continuation lines with tabs.
+
+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
 ------------------
 

CMakeLists.txt

@@ -1,35 +1,48 @@
-cmake_minimum_required(VERSION 3.9)
+cmake_minimum_required(VERSION 3.11)
 
 project(
 	opustags
-	VERSION 1.3.0
+	VERSION 1.10.1
 	LANGUAGES CXX
 )
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 20)
 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}" ${OGG_INCLUDE_DIRS})
+include_directories(BEFORE src "${CMAKE_BINARY_DIR}" ${OGG_INCLUDE_DIRS} ${Iconv_INCLUDE_DIRS})
 
 add_library(
 	ot
 	STATIC
+	src/base64.cc
 	src/cli.cc
 	src/ogg.cc
 	src/opus.cc
 	src/system.cc
 )
-target_link_libraries(ot PUBLIC ${OGG_LIBRARIES})
-
-if (APPLE)
-	target_link_libraries(ot PUBLIC iconv)
-endif()
+target_link_libraries(ot PUBLIC ${OGG_LIBRARIES} ${Iconv_LIBRARIES})
 
 add_executable(opustags src/opustags.cc)
 target_link_libraries(opustags ot)
@@ -38,5 +51,6 @@ include(GNUInstallDirs)
 install(TARGETS opustags DESTINATION "${CMAKE_INSTALL_BINDIR}")
 configure_file(opustags.1 . @ONLY)
 install(FILES "${CMAKE_BINARY_DIR}/opustags.1" DESTINATION "${CMAKE_INSTALL_MANDIR}/man1")
+install(FILES CHANGELOG.md CONTRIBUTING.md LICENSE README.md DESTINATION ${CMAKE_INSTALL_DOCDIR})
 
 add_subdirectory(t)

CONTRIBUTING.md

@@ -25,6 +25,9 @@ You should check that your changes don't break the test suite by running
 Following these practices is important to keep the history clean, and to allow
 for better code reviews.
 
+You can submit pull requests on GitHub at <https://github.com/fmang/opustags>,
+or email me your patches at <fmang+opustags@mg0.fr>.
+
 ## History of opustags
 
 opustags is originally a small project made to fill a need to edit tags in Opus
@@ -42,13 +45,15 @@ 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.
 
 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.
 
+Subsequent releases have been adding new features.
+
 ## Candidate features
 
 The code contains a few `\todo` markers where something could be improved in the
@@ -59,14 +64,7 @@ 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.
-- Skip encoding conversion with --raw.
-- 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.

LICENSE

@@ -1,4 +1,4 @@
-Copyright (c) 2013-2018, Frédéric Mangano-Tarumi
+Copyright (c) 2013-2024, Frédéric Mangano
 All rights reserved.
 
 Redistribution and use in source and binary forms, with or without modification,

README.md

@@ -3,26 +3,28 @@ opustags
 
 View and edit Ogg Opus comments.
 
+opustags supports the following features:
+
+- interactive editing using your preferred text editor,
+- batch editing with command-line flags,
+- tags exporting and importing through text files.
+
 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.
 
-It currently has the following limitations:
+opustags is tag-agnostic: you can write arbitrary key-value tags, and none of them will be treated
+specially. After all, common tags like TITLE or ARTIST are nothing more than conventions.
 
-- 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`.
-
-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.
+The project’s homepage is located at <https://github.com/fmang/opustags>.
 
 Requirements
 ------------
 
 * a POSIX-compliant system,
-* a C++14 compiler,
-* CMake ≥ 3.9,
+* a C++20 compiler,
+* CMake ≥ 3.11,
 * libogg 1.3.3.
 
 The version numbers are indicative, and it's very likely opustags will build and work fine with
@@ -48,17 +50,25 @@ 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             specify the output file
-      -i, --in-place                overwrite the input 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
+      --output-cover FILE           extract and save the cover art, if any
+      --set-cover FILE              sets the cover art
+      --vendor                      print the vendor string
+      --set-vendor VALUE            set the vendor string
+      --raw                         disable encoding conversion
+      -z                            delimit tags with NUL
 
 See the man page, `opustags.1`, for extensive documentation.

opustags.1

@@ -1,4 +1,4 @@
-.TH opustags 1 "December 2018" "@PROJECT_NAME@ @PROJECT_VERSION@"
+.TH opustags 1 "March 2025" "@PROJECT_NAME@ @PROJECT_VERSION@"
 .SH NAME
 opustags \- Ogg Opus tag editor
 .SH SYNOPSIS
@@ -10,21 +10,26 @@ opustags \- Ogg Opus tag editor
 .br
 .B opustags
 .I OPTIONS
+.B -i
+\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 Ogg Opus file.
-It basically has two modes: read-only, and read-write for tag editing.
+It 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.
+printed on standard output. Lines prefixed by tabs are continuation of the previous tag.
 \fIINPUT\fP can either be the name of a file or \fB-\fP to read from standard input.
 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 editing mode, you need to specify an output file with \fB--output\fP, or use \fB--in-place\fP to
-overwrite the input file. If the output is a regular file, the result is first written to a
+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
@@ -38,14 +43,13 @@ to set new tags without being bothered by the old ones.
 If you want to replace all the tags, you can use the \fB--set-all\fP option which will cause
 \fBopustags\fP to read tags from standard input.
 The format is the same as the one used for output: newline-separated \fIFIELD=Value\fP assignment.
-All the previously existing tags as deleted.
+All the previously existing tags are deleted.
 .PP
-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.
+The Opus format specification requires that tags are encoded in UTF-8, so that’s the only encoding
+\fBopustags\fP supports. If your system encoding is different, the tags are automatically converted
+to and from your system locale. When you edit an Opus file whose tags contain characters unsupported
+by your system encoding, the original UTF-8 values will be preserved for the tags you don’t
+explicitly modify.
 .SH OPTIONS
 .TP
 .B \-h, \-\-help
@@ -63,7 +67,7 @@ setting \fB--output\fP to the same path as the input file and enabling \fB--over
 This option conflicts with \fB--output\fP.
 .TP
 .B \-y, \-\-overwrite
-By default, \fBopustags\fP refuses to overwrite an already-existent file.
+By default, \fBopustags\fP refuses to overwrite an already-existing 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
@@ -75,10 +79,10 @@ In both cases, the field names are case-insensitive, and expected to be ASCII.
 .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
 multiple fields with the same name, and previously existing tags will also be preserved.
-When the \fB--delete\fP is used with the same \fIFIELD\fP, only the older tags are deleted.
+When \fB--delete\fP is used with the same \fIFIELD\fP, only the older tags are deleted.
 .TP
 .B \-s, \-\-set \fIFIELD=VALUE\fP
-This option is provided for convenience. It delete all the fields of the same
+This option is provided for convenience. It deletes all the fields of the same
 type that may already exist, then adds it with the wanted value.
 This is strictly equivalent to \fB--delete\fP \fIFIELD\fP \fB--add\fP
 \fIFIELD=VALUE\fP. You can combine it with \fB--add\fP to add tags of the same
@@ -89,10 +93,59 @@ added with \fB--add\fP.
 Delete all the previously existing tags.
 .TP
 .B \-S, \-\-set-all
-Sets the tags from scratch.
+Set 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.
-Blank lines are ignored.
+Empty lines and lines starting with \fI#\fP are ignored.
+Multi-line tags must have their continuation lines prefixed by a single tab (in other words, every
+\fI\\n\fP must be replaced by \fI\\n\\t\fP).
+.TP
+.B \-e, \-\-edit
+Edit tags interactively by spawning the program specified by the EDITOR
+environment variable. The allowed format is the same as with \fB--set-all\fP.
+If TERM and VISUAL are set, VISUAL takes precedence over EDITOR.
+.TP
+.B \-\-output-cover \fIFILE\fP
+Extract the cover art from the \fBMETADATA_BLOCK_PICTURE\fP tag into the specified location.
+If the input file does not contain any cover art, this option has no effect.
+To allow overwriting the target location, specify \fB--overwrite\fP.
+In the case of multiple pictures embedded in the Opus tags, only the first one is saved.
+Note that since the image format is not fixed, you should consider an extension-less file name and
+rely on the magic number to deduce the type.
+\fBopustags\fP does not add or check the target file’s extension.
+You can specify \fB-\fP for standard output, in which case the regular output will be suppressed.
+.TP
+.B \-\-set-cover \fIFILE\fP
+Set the cover art by embedding the specified picture into the \fBMETADATA_BLOCK_PICTURE\fP tag,
+replacing any existing values.
+Specify \fB-\fP to read the picture from standard input.
+In theory, an Opus file may contain multiple pictures with different roles, though in practice only
+the front cover really matters.
+\fBopustags\fP can currently only handle one front cover and nothing else.
+.TP
+.B \-\-vendor
+Print the vendor string from the OpusTags packet and do nothing else. Standard tags operations are
+not supported when specifying this flag.
+.TP
+.B \-\-set-vendor \fIVALUE\fP
+Replace the vendor string by the specified value. This action can be performed alongside tag
+edition.
+.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.
+.TP
+.B \-z
+When editing tags programmatically with line-based tools like grep or sed, tags containing newlines
+are likely to corrupt the result because these tools won’t interpret multi-line tags as a whole. To
+make automatic processing easier, \fB-z\fP delimits tags by a null byte (ASCII NUL) instead of line
+feeds. That same \fB-z\fP flag is also supported by GNU grep or GNU sed and, combined with
+\fBopustags -z\fP, would make them process the input tag-by-tag instead of line-by-line, thus
+supporting multi-line tags as well.
+This option also disables the tab prefix for continuation lines after a line feed.
 .SH EXAMPLES
 .PP
 List all the tags in file foo.opus:
@@ -103,25 +156,29 @@ Copy in.opus to out.opus, with the TITLE tag added:
 .PP
 	opustags in.opus --output out.opus --add "TITLE=Hello world!"
 .PP
-Replace all the tags in dest.opus with the ones from src.opus:
-.PP
-	opustags src.opus | opustags --in-place dest.opus --set-all
-.PP
 Remove the previously existing ARTIST tags and add the two X and Y ARTIST tags, then display the new
 tags without writing them to the Opus file:
 .PP
 	opustags in.opus --add ARTIST=X --add ARTIST=Y --delete ARTIST
+.PP
+Edit tags interactively in Vim:
+.PP
+	EDITOR=vim opustags --in-place --edit file.opus
+.PP
+Replace all the tags in dest.opus with the ones from src.opus:
+.PP
+	opustags src.opus | opustags --in-place dest.opus --set-all
+.PP
+Use GNU grep to remove all the CHAPTER* tags, with -z to support multi-line tags:
+.PP
+	opustags -z file.opus | grep -z -v ^CHAPTER | opustags -z --in-place file.opus --set-all
 .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]
+.IP \[bu] 2n
 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.
+Control characters inside tags are printed raw rather than being escaped.
 .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
@@ -130,6 +187,6 @@ applies for the vendor string.
 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+opustags@mg0.fr>
+Frédéric Mangano <fmang+opustags@mg0.fr>
 .PP
 Report bugs at <https://github.com/fmang/opustags/issues>

src/base64.cc

@@ -0,0 +1,97 @@
+/**
+ * \file src/base64.cc
+ * \brief Base64 encoding/decoding (RFC 4648).
+ *
+ * Inspired by Jouni Malinen’s BSD implementation at
+ * <http://web.mit.edu/freebsd/head/contrib/wpa/src/utils/base64.c>.
+ *
+ * This implementation is used to decode the cover arts embedded in the tags. According to
+ * <https://wiki.xiph.org/VorbisComment>, line feeds are not allowed and padding is required.
+ */
+
+#include <opustags.h>
+
+#include <string.h>
+
+static const char8_t base64_table[65] =
+	u8"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+
+std::u8string ot::encode_base64(ot::byte_string_view src)
+{
+	size_t len = src.size();
+	size_t num_blocks = (len + 2) / 3; // Count of 3-byte blocks, rounded up.
+	size_t olen = num_blocks * 4; // Each 3-byte block becomes 4 base64 bytes.
+	if (olen < len)
+		throw std::overflow_error("failed to encode excessively long base64 block");
+
+	std::u8string out;
+	out.resize(olen);
+
+	const uint8_t* in = src.data();
+	const uint8_t* end = in + len;
+	char8_t* pos = out.data();
+	while (end - in >= 3) {
+		*pos++ = base64_table[in[0] >> 2];
+		*pos++ = base64_table[((in[0] & 0x03) << 4) | (in[1] >> 4)];
+		*pos++ = base64_table[((in[1] & 0x0f) << 2) | (in[2] >> 6)];
+		*pos++ = base64_table[in[2] & 0x3f];
+		in += 3;
+	}
+
+	if (end - in) {
+		*pos++ = base64_table[in[0] >> 2];
+		if (end - in == 1) {
+			*pos++ = base64_table[(in[0] & 0x03) << 4];
+			*pos++ = '=';
+		} else { // end - in == 2
+			*pos++ = base64_table[((in[0] & 0x03) << 4) | (in[1] >> 4)];
+			*pos++ = base64_table[(in[1] & 0x0f) << 2];
+		}
+		*pos++ = '=';
+	}
+
+	return out;
+}
+
+ot::byte_string ot::decode_base64(std::u8string_view src)
+{
+	// Remove the padding and rely on the string length instead.
+	while (!src.empty() && src.back() == u8'=')
+		src.remove_suffix(1);
+
+	size_t olen = src.size() / 4 * 3; // Whole blocks;
+	switch (src.size() % 4) {
+		case 1: throw status {st::error, "invalid base64 block size"};
+		case 2: olen += 1; break;
+		case 3: olen += 2; break;
+	}
+
+	ot::byte_string out;
+	out.resize(olen);
+	uint8_t* pos = out.data();
+
+	unsigned char dtable[256];
+	memset(dtable, 0x80, 256);
+	for (size_t i = 0; i < sizeof(base64_table) - 1; ++i)
+		dtable[(size_t) base64_table[i]] = (unsigned char) i;
+
+	unsigned char block[4];
+	size_t count = 0;
+	for (unsigned char c : src) {
+		unsigned char tmp = dtable[c];
+		if (tmp == 0x80)
+			throw status {st::error, "invalid base64 character"};
+
+		block[count++] = tmp;
+		if (count == 2) {
+			*pos++ = (block[0] << 2) | (block[1] >> 4);
+		} else if (count == 3) {
+			*pos++ = (block[1] << 4) | (block[2] >> 2);
+		} else if (count == 4) {
+			*pos++ = (block[2] << 6) | block[3];
+			count = 0;
+		}
+	}
+
+	return out;
+}

src/cli.cc

@@ -4,21 +4,18 @@
  *
  * 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>
-
-using namespace std::literals::string_literals;
+#include <unistd.h>
+#include <algorithm>
 
 static const char help_message[] =
 PROJECT_NAME " version " PROJECT_VERSION
@@ -26,18 +23,26 @@ R"raw(
 
 Usage: opustags --help
        opustags [OPTIONS] FILE
+       opustags OPTIONS -i FILE...
        opustags OPTIONS FILE -o FILE
 
 Options:
   -h, --help                    print this help
   -o, --output FILE             specify the output file
-  -i, --in-place                overwrite the input 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
+  --output-cover FILE           extract and save the cover art, if any
+  --set-cover FILE              sets the cover art
+  --vendor                      print the vendor string
+  --set-vendor VALUE            set the vendor string
+  --raw                         disable encoding conversion
+  -z                            delimit tags with NUL
 
 See the man page for extensive documentation.
 )raw";
@@ -52,173 +57,318 @@ 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'},
+	{"output-cover", required_argument, 0, 'c'},
+	{"set-cover", required_argument, 0, 'C'},
+	{"vendor", no_argument, 0, 'v'},
+	{"set-vendor", required_argument, 0, 'V'},
+	{"raw", no_argument, 0, 'r'},
 	{NULL, 0, 0, 0}
 };
 
-ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
+ot::options ot::parse_options(int argc, char** argv, FILE* comments_input)
 {
-	static ot::encoding_converter to_utf8("", "UTF-8");
-	std::string utf8;
-	std::string::size_type equal;
+	options opt;
+	const char* equal;
 	ot::status rc;
+	std::list<std::string> local_to_add; // opt.to_add before UTF-8 conversion.
+	std::list<std::string> local_to_delete; // opt.to_delete before UTF-8 conversion.
+	bool set_all = false;
+	std::optional<std::string> set_cover;
+	std::optional<std::string> set_vendor;
 	opt = {};
 	if (argc == 1)
-		return {st::bad_arguments, "No arguments specified. Use -h for help."};
-	bool in_place = false;
+		throw status {st::bad_arguments, "No arguments specified. Use -h for help."};
 	int c;
 	optind = 0;
-	while ((c = getopt_long(argc, argv, ":ho:iyd:a:s:DS", getopt_options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, ":ho:iyd:a:s:DSez", getopt_options, NULL)) != -1) {
 		switch (c) {
 		case 'h':
 			opt.print_help = true;
 			break;
 		case 'o':
-			if (!opt.path_out.empty())
-				return {st::bad_arguments, "Cannot specify --output more than once."};
+			if (opt.path_out)
+				throw status {st::bad_arguments, "Cannot specify --output more than once."};
 			opt.path_out = optarg;
-			if (opt.path_out.empty())
-				return {st::bad_arguments, "Output file path cannot be empty."};
 			break;
 		case 'i':
-			in_place = true;
+			opt.in_place = true;
+			opt.overwrite = true;
 			break;
 		case 'y':
 			opt.overwrite = true;
 			break;
 		case 'd':
-			rc = to_utf8(optarg, strlen(optarg), utf8);
-			if (rc != ot::st::ok)
-				return {st::bad_arguments, "Could not encode argument into UTF-8: " + rc.message};
-			opt.to_delete.emplace_back(std::move(utf8));
+			local_to_delete.emplace_back(optarg);
 			break;
 		case 'a':
 		case 's':
-			rc = to_utf8(optarg, strlen(optarg), utf8);
-			if (rc != ot::st::ok)
-				return {st::bad_arguments, "Could not encode argument into UTF-8: " + rc.message};
-			if ((equal = utf8.find('=')) == std::string::npos)
-				return {st::bad_arguments, "Comment does not contain an equal sign: "s + optarg + "."};
+			equal = strchr(optarg, '=');
+			if (equal == nullptr)
+				throw status {st::bad_arguments, "Comment does not contain an equal sign: "s + optarg + "."};
 			if (c == 's')
-				opt.to_delete.emplace_back(utf8.substr(0, equal));
-			opt.to_add.emplace_back(std::move(utf8));
+				local_to_delete.emplace_back(optarg, equal - optarg);
+			local_to_add.emplace_back(optarg);
 			break;
 		case 'S':
-			opt.set_all = true;
+			opt.delete_all = true;
+			set_all = true;
 			break;
 		case 'D':
 			opt.delete_all = true;
 			break;
+		case 'e':
+			opt.edit_interactively = true;
+			break;
+		case 'c':
+			if (opt.cover_out)
+				throw status {st::bad_arguments, "Cannot specify --output-cover more than once."};
+			opt.cover_out = optarg;
+			break;
+		case 'C':
+			if (set_cover)
+				throw status {st::bad_arguments, "Cannot specify --set-cover more than once."};
+			set_cover = optarg;
+			break;
+		case 'v':
+			opt.print_vendor = true;
+			break;
+		case 'V':
+			if (set_vendor)
+				throw status {st::bad_arguments, "Cannot specify --set-vendor more than once."};
+			set_vendor = optarg;
+			break;
+		case 'r':
+			opt.raw = true;
+			break;
+		case 'z':
+			opt.tag_delimiter = '\0';
+			break;
 		case ':':
-			return {st::bad_arguments,
-			        "Missing value for option '"s + argv[optind - 1] + "'."};
+			throw status {st::bad_arguments, "Missing value for option '"s + argv[optind - 1] + "'."};
 		default:
-			return {st::bad_arguments, "Unrecognized option '" +
-			        (optopt ? "-"s + static_cast<char>(optopt) : argv[optind - 1]) + "'."};
+			throw status {st::bad_arguments, "Unrecognized option '" +
+			              (optopt ? "-"s + static_cast<char>(optopt) : argv[optind - 1]) + "'."};
 		}
 	}
 	if (opt.print_help)
-		return st::ok;
-	if (optind != argc - 1)
-		return {st::bad_arguments, "Exactly one input file must be specified."};
-	opt.path_in = argv[optind];
-	if (opt.path_in.empty())
-		return {st::bad_arguments, "Input file path cannot be empty."};
-	if (in_place) {
-		if (!opt.path_out.empty())
-			return {st::bad_arguments, "Cannot combine --in-place and --output."};
-		if (opt.path_in == "-")
-			return {st::bad_arguments, "Cannot modify standard input in place."};
-		opt.path_out = opt.path_in;
-		opt.overwrite = true;
+		return opt;
+
+	// All non-option arguments are input files.
+	size_t stdin_uses = 0;
+	for (int i = optind; i < argc; i++) {
+		if (strcmp(argv[i], "-") == 0)
+			++stdin_uses;
+		opt.paths_in.emplace_back(argv[i]);
 	}
-	if (opt.path_in == "-" && opt.set_all)
-		return {st::bad_arguments,
-		        "Cannot use standard input as input file when --set-all is specified."};
-	return st::ok;
+	bool stdin_as_input = stdin_uses > 0;
+
+	if (set_cover == "-")
+		++stdin_uses;
+	if (set_all)
+		++stdin_uses;
+	if (stdin_uses > 1)
+		throw status { st::bad_arguments, "Cannot use standard input more than once." };
+
+	// Convert arguments to UTF-8.
+	if (opt.raw) {
+		// Cast the user data without any encoding conversion.
+		auto cast_to_utf8 = [](std::string_view in)
+			{ return std::u8string(reinterpret_cast<const char8_t*>(in.data()), in.size()); };
+		std::transform(local_to_add.begin(), local_to_add.end(),
+			       std::back_inserter(opt.to_add), cast_to_utf8);
+		std::transform(local_to_delete.begin(), local_to_delete.end(),
+			       std::back_inserter(opt.to_delete), cast_to_utf8);
+		if (set_vendor)
+			opt.set_vendor = cast_to_utf8(*set_vendor);
+	} else {
+		try {
+			std::transform(local_to_add.begin(), local_to_add.end(),
+			               std::back_inserter(opt.to_add), encode_utf8);
+			std::transform(local_to_delete.begin(), local_to_delete.end(),
+			               std::back_inserter(opt.to_delete), encode_utf8);
+			if (set_vendor)
+				opt.set_vendor = encode_utf8(*set_vendor);
+		} catch (const ot::status& rc) {
+			throw status {st::bad_arguments, "Could not encode argument into UTF-8: " + rc.message};
+		}
+	}
+
+	bool read_only = !opt.in_place && !opt.path_out.has_value();
+
+	if (opt.in_place && opt.path_out)
+		throw status {st::bad_arguments, "Cannot combine --in-place and --output."};
+
+	if (opt.in_place && stdin_as_input)
+		throw status {st::bad_arguments, "Cannot modify standard input in place."};
+
+	if ((!opt.in_place || opt.edit_interactively) && opt.paths_in.size() != 1)
+		throw status {st::bad_arguments, "Exactly one input file must be specified."};
+
+	if (opt.edit_interactively && (stdin_as_input || opt.path_out == "-" || opt.cover_out == "-"))
+		throw status {st::bad_arguments, "Cannot edit interactively when standard input or standard output are already used."};
+
+	if (opt.edit_interactively && read_only)
+		throw status {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()))
+		throw status {st::bad_arguments, "Cannot mix --edit with -adDsS."};
+
+	if (opt.cover_out == "-" && opt.path_out == "-")
+		throw status {st::bad_arguments, "Cannot specify standard output for both --output and --output-cover."};
+
+	if (opt.cover_out && opt.paths_in.size() > 1)
+		throw status {st::bad_arguments, "Cannot use --output-cover with multiple input files."};
+
+	if (opt.print_vendor && !read_only)
+		throw status {st::bad_arguments, "--vendor is only supported in read-only mode."};
+
+	if (set_cover) {
+		byte_string picture_data = ot::slurp_binary_file(set_cover->c_str());
+		opt.to_delete.push_back(u8"METADATA_BLOCK_PICTURE"s);
+		opt.to_add.push_back(ot::make_cover(picture_data));
+	}
+
+	if (set_all) {
+		// Read comments from stdin and prepend them to opt.to_add.
+		std::list<std::u8string> comments = read_comments(comments_input, opt);
+		opt.to_add.splice(opt.to_add.begin(), std::move(comments));
+	}
+	return opt;
+}
+
+/** Format a UTF-8 string by adding tabulations (\t) after line feeds (\n) to mark continuation for
+ *  multiline values. With -z, this behavior applies for embedded NUL characters instead of LF. */
+static std::u8string format_value(const std::u8string& source, const ot::options& opt)
+{
+	auto newline_count = std::count(source.begin(), source.end(), opt.tag_delimiter);
+
+	// General case: the value fits on a single line. Use std::string’s copy constructor for the
+	// most efficient copy we could hope for.
+	if (newline_count == 0)
+		return source;
+
+	std::u8string formatted;
+	formatted.reserve(source.size() + newline_count);
+	for (auto c : source) {
+		formatted.push_back(c);
+		if (c == opt.tag_delimiter)
+			formatted.push_back(u8'\t');
+	}
+	return formatted;
 }
 
 /**
- * \todo Escape new lines.
+ * Convert the comment from UTF-8 to the system encoding if relevant, and print it with a trailing
+ * line feed.
  */
-void ot::print_comments(const std::list<std::string>& comments, FILE* output)
+static void puts_utf8(std::u8string_view str, FILE* output, const ot::options& opt)
 {
-	static ot::encoding_converter from_utf8("UTF-8", "//TRANSLIT");
-	std::string local;
-	bool info_lost = false;
-	bool bad_comments = false;
-	bool has_newline = false;
-	bool has_control = false;
-	for (const std::string& comment : comments) {
-		ot::status rc = from_utf8(comment, local);
-		if (rc == ot::st::information_lost) {
-			info_lost = true;
-		} else if (rc != ot::st::ok) {
-			bad_comments = true;
-			continue;
+	if (opt.raw) {
+		fwrite(str.data(), 1, str.size(), output);
+	} else {
+		try {
+			std::string local = ot::decode_utf8(str);
+			fwrite(local.data(), 1, local.size(), output);
+		} catch (ot::status& rc) {
+			rc.message += " See --raw.";
+			throw;
 		}
-		for (unsigned char c : comment) {
-			if (c == '\n')
-				has_newline = true;
-			else if (c < 0x20)
-				has_control = true;
-		}
-		fwrite(local.data(), 1, local.size(), output);
-		putchar('\n');
 	}
-	if (info_lost)
-		fputs("warning: Some tags have been transliterated to your system encoding.\n", stderr);
-	if (bad_comments)
-		fputs("warning: Some tags are not properly encoded and have not been displayed.\n", stderr);
-	if (has_newline)
-		fputs("warning: Some tags contain newline characters. "
-		      "These are not supported by --set-all.\n", stderr);
+	putc(opt.tag_delimiter, output);
+}
+
+/**
+ * Print comments in a human readable format that can also be read back in by #read_comment.
+ *
+ * To disambiguate between a newline embedded in a comment and a newline representing the start of
+ * the next tag, continuation lines always have a single TAB (^I) character added to the beginning.
+ */
+void ot::print_comments(const std::list<std::u8string>& comments, FILE* output, const ot::options& opt)
+{
+	bool has_control = false;
+	for (const std::u8string& source_comment : comments) {
+		if (!has_control) { // Don’t bother analyzing comments if the flag is already up.
+			for (unsigned char c : source_comment) {
+				if (c < 0x20 && c != '\n') {
+					has_control = true;
+					break;
+				}
+			}
+		}
+		std::u8string utf8_comment = format_value(source_comment, opt);
+		puts_utf8(utf8_comment, output, opt);
+	}
 	if (has_control)
 		fputs("warning: Some tags contain control characters.\n", stderr);
 }
 
-ot::status ot::read_comments(FILE* input, std::list<std::string>& comments)
+std::list<std::u8string> ot::read_comments(FILE* input, const ot::options& opt)
 {
-	static ot::encoding_converter to_utf8("", "UTF-8");
+	std::list<std::u8string> comments;
 	comments.clear();
-	char* line = nullptr;
+	char* source_line = nullptr;
 	size_t buflen = 0;
 	ssize_t nread;
-	while ((nread = getline(&line, &buflen, input)) != -1) {
-		if (nread > 0 && line[nread - 1] == '\n')
-			--nread;
-		if (nread == 0)
-			continue;
-		if (memchr(line, '=', nread) == nullptr) {
-			ot::status rc = {ot::st::error, "Malformed tag: " + std::string(line, nread)};
-			free(line);
-			return rc;
-		}
-		std::string utf8;
-		ot::status rc = to_utf8(line, nread, utf8);
-		if (rc == ot::st::ok) {
-			comments.emplace_back(std::move(utf8));
+	std::u8string* previous_comment = nullptr;
+	while ((nread = getdelim(&source_line, &buflen, opt.tag_delimiter, input)) != -1) {
+		if (nread > 0 && source_line[nread - 1] == opt.tag_delimiter)
+			--nread; // Chomp.
+
+		std::u8string line;
+		if (opt.raw) {
+			line = std::u8string(reinterpret_cast<char8_t*>(source_line), nread);
 		} else {
-			free(line);
-			return {ot::st::badly_encoded, "UTF-8 conversion error: " + rc.message};
+			try {
+				line = encode_utf8(std::string_view(source_line, nread));
+			} catch (const ot::status& rc) {
+				free(source_line);
+				throw ot::status {ot::st::badly_encoded, "UTF-8 conversion error: " + rc.message};
+			}
+		}
+
+		if (line.empty()) {
+			// Ignore empty lines.
+			previous_comment = nullptr;
+		} else if (line[0] == u8'#') {
+			// Ignore comments.
+			previous_comment = nullptr;
+		} else if (line[0] == u8'\t') {
+			// Continuation line: append the current line to the previous tag.
+			if (previous_comment == nullptr) {
+				ot::status rc = {ot::st::error, "Unexpected continuation line: " + std::string(source_line, nread)};
+				free(source_line);
+				throw rc;
+			} else {
+				line[0] = opt.tag_delimiter;
+				previous_comment->append(line);
+			}
+		} else if (line.find(u8'=') == decltype(line)::npos) {
+			ot::status rc = {ot::st::error, "Malformed tag: " + std::string(source_line, nread)};
+			free(source_line);
+			throw rc;
+		} else {
+			previous_comment = &comments.emplace_back(std::move(line));
 		}
 	}
-	free(line);
-	return ot::st::ok;
+	free(source_line);
+	return comments;
 }
 
-void ot::delete_comments(std::list<std::string>& comments, const std::string& selector)
+void ot::delete_comments(std::list<std::u8string>& comments, const std::u8string& selector)
 {
 	auto name = selector.data();
-	auto equal = selector.find('=');
-	auto value = (equal == std::string::npos ? nullptr : name + equal + 1);
+	auto equal = selector.find(u8'=');
+	auto value = (equal == std::u8string::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++;
+		/** \todo Avoid using strncasecmp because it assumes the system locale is UTF-8. */
 		bool name_match = current->size() > name_len + 1 &&
 		                  (*current)[name_len] == '=' &&
-		                  strncasecmp(current->data(), name, name_len) == 0;
+		                  strncasecmp((const char*) current->data(), (const char*) name, name_len) == 0;
 		if (!name_match)
 			continue;
 		bool value_match = value == nullptr ||
@@ -230,22 +380,112 @@ void ot::delete_comments(std::list<std::string>& comments, const std::string& se
 }
 
 /** Apply the modifications requested by the user to the opustags packet. */
-static ot::status edit_tags(ot::opus_tags& tags, const ot::options& opt)
+static void edit_tags(ot::opus_tags& tags, const ot::options& opt)
 {
-	if (opt.set_all) {
-		auto rc = ot::read_comments(stdin, tags.comments);
-		if (rc != ot::st::ok)
-			return rc;
-	} else if (opt.delete_all) {
+	if (opt.set_vendor)
+		tags.vendor = *opt.set_vendor;
+
+	if (opt.delete_all) {
 		tags.comments.clear();
-	} else for (const std::string& name : opt.to_delete) {
-		ot::delete_comments(tags.comments, name.c_str());
+	} else for (const std::u8string& name : opt.to_delete) {
+		ot::delete_comments(tags.comments, name);
 	}
 
-	for (const std::string& comment : opt.to_add)
+	for (const std::u8string& comment : opt.to_add)
 		tags.comments.emplace_back(comment);
+}
 
-	return ot::st::ok;
+/** Spawn VISUAL or EDITOR to edit the given tags. */
+static void edit_tags_interactively(ot::opus_tags& tags, const std::optional<std::string>& base_path, const ot::options& opt)
+{
+	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)
+		throw ot::status {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)
+		throw ot::status {ot::st::standard_error,
+		                  "Could not open '" + tags_path + "': " + strerror(errno)};
+	ot::print_comments(tags.comments, tags_file.get(), opt);
+	tags_file.reset();
+
+	// Spawn the editor, and watch the modification timestamps.
+	timespec before = ot::get_file_timestamp(tags_path.c_str());
+	ot::status editor_rc;
+	try {
+		ot::run_editor(editor, tags_path);
+		editor_rc = ot::st::ok;
+	} catch (const ot::status& rc) {
+		editor_rc = rc;
+	}
+	timespec after = ot::get_file_timestamp(tags_path.c_str());
+	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());
+		throw editor_rc;
+	} else if (!modified) {
+		remove(tags_path.c_str());
+		fputs("Cancelling edition because the tags file was not modified.\n", stderr);
+		throw ot::status {ot::st::cancel};
+	}
+
+	// Applying the new tags.
+	tags_file = fopen(tags_path.c_str(), "re");
+	if (tags_file == nullptr)
+		throw ot::status {ot::st::standard_error, "Error opening " + tags_path + ": " + strerror(errno)};
+	try {
+		tags.comments = ot::read_comments(tags_file.get(), opt);
+	} catch (const ot::status& rc) {
+		fprintf(stderr, "warning: Leaving %s on the disk.\n", tags_path.c_str());
+		throw;
+	}
+	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());
+}
+
+static void output_cover(const ot::opus_tags& tags, const ot::options &opt)
+{
+	std::optional<ot::picture> cover = extract_cover(tags);
+	if (!cover) {
+		fputs("warning: No cover found.\n", stderr);
+		return;
+	}
+
+	ot::file output;
+	if (opt.cover_out == "-") {
+		output = stdout;
+	} else {
+		struct stat output_info;
+		if (stat(opt.cover_out->c_str(), &output_info) == 0) {
+			if (S_ISREG(output_info.st_mode) && !opt.overwrite)
+				throw ot::status {ot::st::error, "'" + opt.cover_out.value() + "' already exists. Use -y to overwrite."};
+		} else if (errno != ENOENT) {
+			throw ot::status {ot::st::error, "Could not identify '" + opt.cover_out.value() + "': " + strerror(errno)};
+		}
+
+		output = fopen(opt.cover_out->c_str(), "w");
+		if (output == nullptr)
+			throw ot::status {ot::st::standard_error, "Could not open '" + opt.cover_out.value() + "' for writing: " + strerror(errno)};
+	}
+
+	if (fwrite(cover->picture_data.data(), 1, cover->picture_data.size(), output.get()) < cover->picture_data.size())
+		throw ot::status {ot::st::standard_error, "fwrite error: "s + strerror(errno)};
 }
 
 /**
@@ -254,82 +494,79 @@ static ot::status edit_tags(ot::opus_tags& tags, const ot::options& opt)
  *
  * The writer is optional. When writer is nullptr, opustags runs in read-only mode.
  */
-static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const ot::options &opt)
+static void process(ot::ogg_reader& reader, ot::ogg_writer* writer, const ot::options &opt)
 {
 	bool focused = false; /*< the stream on which we operate is defined */
 	int focused_serialno; /*< when focused, the serialno of the focused stream */
-	/** \todo Become stream-aware instead of counting the pages of all streams together. */
-	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;
+
+	/** When the number of pages the OpusTags packet takes differs from the input stream to the
+	 *  output stream, we need to renumber all the succeeding pages. If the input stream
+	 *  contains gaps, the offset will naively reproduce the gaps: page numbers 0 (1) 2 4 will
+	 *  become 0 (1 2) 3 5, where (…) is the OpusTags packet, and not 0 (1 2) 3 4. */
+	int pageno_offset = 0;
+
+	while (reader.next_page()) {
 		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) {
-			return {ot::st::error, "Muxed streams are not supported yet."};
+			/** \todo Support mixed streams. */
+			throw ot::status {ot::st::error, "Muxed streams are not supported yet."};
 		}
-		if (absolute_page_no == 0) { // Identification header
+		if (reader.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
+				throw ot::status {ot::st::error, "Not an Opus stream."};
+			if (writer)
+				writer->write_page(reader.page);
+		} else if (reader.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;
+			reader.process_header_packet([&tags](ogg_packet& p) { tags = ot::parse_tags(p); });
+			if (opt.cover_out)
+				output_cover(tags, opt);
+			edit_tags(tags, opt);
 			if (writer) {
+				if (opt.edit_interactively) {
+					fflush(writer->file); // flush before calling the subprocess
+					edit_tags_interactively(tags, writer->path, opt);
+				}
 				auto packet = ot::render_tags(tags);
-				rc = writer->write_header_packet(serialno, pageno, packet);
-				if (rc != ot::st::ok)
-					return rc;
+				writer->write_header_packet(serialno, pageno, packet);
+				pageno_offset = writer->next_page_no - 1 - reader.absolute_page_no;
 			} else {
-				ot::print_comments(tags.comments, stdout);
+				if (opt.cover_out != "-") {
+					if (opt.print_vendor)
+						puts_utf8(tags.vendor, stdout, opt);
+					else
+						ot::print_comments(tags.comments, stdout, opt);
+				}
 				break;
 			}
-		} else {
-			if (writer && (rc = writer->write_page(reader.page)) != ot::st::ok)
-				return rc;
+		} else if (writer) {
+			ot::renumber_page(reader.page, pageno + pageno_offset);
+			writer->write_page(reader.page);
 		}
 	}
-	if (absolute_page_no < 1)
-		return {ot::st::error, "Expected at least 2 Ogg pages."};
-	return ot::st::ok;
+	if (reader.absolute_page_no < 1)
+		throw ot::status {ot::st::error, "Expected at least 2 Ogg pages."};
 }
 
-ot::status ot::run(const ot::options& opt)
+static void run_single(const ot::options& opt, const std::string& path_in, const std::optional<std::string>& path_out)
 {
-	if (opt.print_help) {
-		fputs(help_message, stdout);
-		return st::ok;
-	}
-
 	ot::file input;
-	if (opt.path_in == "-")
+	if (path_in == "-")
 		input = stdin;
-	else if ((input = fopen(opt.path_in.c_str(), "r")) == nullptr)
-		return {ot::st::standard_error,
-		        "Could not open '" + opt.path_in + "' for reading: " + strerror(errno)};
+	else if ((input = fopen(path_in.c_str(), "re")) == nullptr)
+		throw ot::status {ot::st::standard_error,
+		                  "Could not open '" + path_in + "' for reading: " + strerror(errno)};
 	ot::ogg_reader reader(input.get());
 
 	/* Read-only mode. */
-	if (opt.path_out.empty())
-		return process(reader, nullptr, opt);
+	if (!path_out) {
+		process(reader, nullptr, opt);
+		return;
+	}
 
 	/* Read-write mode.
 	 *
@@ -339,11 +576,10 @@ ot::status ot::run(const ot::options& opt)
 	 *  - temporary_output.get() for regular files.
 	 *
 	 * We use a temporary output file for the following reasons:
-	 *  1. The partial .opus output may be seen by softwares like media players, or through
-	 *     inotify for the most attentive process.
+	 *  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 still going to stay but will have an
-	 *     obvious name.
+	 *     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.
@@ -355,40 +591,57 @@ ot::status ot::run(const ot::options& opt)
 	ot::partial_file temporary_output;
 	ot::file final_output;
 
-	ot::status rc = ot::st::ok;
 	struct stat output_info;
-	if (opt.path_out == "-") {
+	if (path_out == "-") {
 		output = stdout;
-	} else if (stat(opt.path_out.c_str(), &output_info) == 0) {
+	} 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(opt.path_out.c_str(), "w")) == nullptr)
-				rc = {ot::st::standard_error,
-				      "Could not open '" + opt.path_out + "' for writing: " +
-				       strerror(errno)};
+			if ((final_output = fopen(path_out->c_str(), "we")) == nullptr)
+				throw ot::status {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(opt.path_out.c_str());
+			temporary_output.open(path_out->c_str());
 			output = temporary_output.get();
 		} else {
-			rc = {ot::st::error,
-			      "'" + opt.path_out + "' already exists. Use -y to overwrite."};
+			throw ot::status {ot::st::error, "'" + path_out.value() + "' already exists. Use -y to overwrite."};
 		}
 	} else if (errno == ENOENT) {
-		rc = temporary_output.open(opt.path_out.c_str());
+		temporary_output.open(path_out->c_str());
 		output = temporary_output.get();
 	} else {
-		rc = {ot::st::error,
-		      "Could not identify '" + opt.path_in + "': " + strerror(errno)};
+		throw ot::status {ot::st::error, "Could not identify '" + path_out.value() + "': " + strerror(errno)};
 	}
-	if (rc != ot::st::ok)
-		return rc;
 
 	ot::ogg_writer writer(output);
-	rc = process(reader, &writer, opt);
-	if (rc == ot::st::ok)
-		rc = temporary_output.commit();
+	writer.path = path_out;
+	process(reader, &writer, opt);
 
-	return rc;
+	// Close the input file and finalize the output. When --in-place is specified, some file
+	// systems like SMB require that the input is closed first.
+	input.reset();
+	temporary_output.commit();
+}
+
+void ot::run(const ot::options& opt)
+{
+	if (opt.print_help) {
+		fputs(help_message, stdout);
+		return;
+	}
+
+	ot::status global_rc = st::ok;
+	for (const auto& path_in : opt.paths_in) {
+		try {
+			run_single(opt, path_in, opt.in_place ? path_in : opt.path_out);
+		} catch (const ot::status& rc) {
+			global_rc = st::error;
+			if (!rc.message.empty())
+				fprintf(stderr, "%s: error: %s\n", path_in.c_str(), rc.message.c_str());
+		}
+	}
+	if (global_rc != st::ok)
+		throw 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

@@ -13,8 +13,6 @@
 #include <errno.h>
 #include <string.h>
 
-using namespace std::literals::string_literals;
-
 bool ot::is_opus_stream(const ogg_page& identification_header)
 {
 	if (ogg_page_bos(&identification_header) == 0)
@@ -24,93 +22,116 @@ bool ot::is_opus_stream(const ogg_page& identification_header)
 	return (memcmp(identification_header.body, "OpusHead", 8) == 0);
 }
 
-ot::status ot::ogg_reader::next_page()
+bool ot::ogg_reader::next_page()
 {
 	int rc;
 	while ((rc = ogg_sync_pageout(&sync, &page)) != 1) {
-		if (rc == -1)
-			return {st::bad_stream, "Unsynced data in stream."};
+		if (rc == -1) {
+			throw status {st::bad_stream,
+			              absolute_page_no == (size_t) -1 ? "Input is not a valid Ogg file."
+			                                              : "Unsynced data in stream."};
+		}
 		if (ogg_sync_check(&sync) != 0)
-			return {st::libogg_error, "ogg_sync_check signalled an error."};
+			throw status {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."};
+				throw status {st::bad_stream, "Unsynced data at end of stream."};
+			return false; // end of sream
 		}
 		char* buf = ogg_sync_buffer(&sync, 65536);
 		if (buf == nullptr)
-			return {st::libogg_error, "ogg_sync_buffer failed."};
+			throw status {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)};
+			throw status {st::standard_error, "fread error: "s + strerror(errno)};
 		if (ogg_sync_wrote(&sync, len) != 0)
-			return {st::libogg_error, "ogg_sync_wrote failed."};
+			throw status {st::libogg_error, "ogg_sync_wrote failed."};
 	}
-	return st::ok;
+	++absolute_page_no;
+	return true;
 }
 
-ot::status ot::ogg_reader::process_header_packet(const std::function<status(ogg_packet&)>& f)
+void ot::ogg_reader::process_header_packet(const std::function<void(ogg_packet&)>& f)
 {
 	if (ogg_page_continued(&page))
-		return {ot::st::error, "Unexpected continued header page."};
+		throw status {ot::st::error, "Unexpected continued header page."};
+
+	ogg_packet packet;
 	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 (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;
+
+	for (;;) {
+		if (ogg_stream_pagein(&stream, &page) != 0)
+			throw status {st::libogg_error, "ogg_stream_pagein failed."};
+
+		int rc = ogg_stream_packetout(&stream, &packet);
+		if (ogg_stream_check(&stream) != 0 || rc == -1) {
+			throw status {ot::st::libogg_error, "ogg_stream_packetout failed."};
+		} else if (rc == 0) {
+			// Not enough data: read the next page.
+			if (!next_page())
+				throw status {ot::st::error, "Unterminated header packet."};
+			continue;
+		} else {
+			// The packet was successfully read.
+			break;
+		}
+	}
+
+	f(packet);
+
 	/* 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;
+		throw status {ot::st::error, "Header page contains more than a single packet."};
 }
 
-ot::status ot::ogg_writer::write_page(const ogg_page& page)
+void ot::ogg_writer::write_page(const ogg_page& page)
 {
 	if (page.header_len < 0 || page.body_len < 0)
-		return {st::int_overflow, "Overflowing page length"};
+		throw status {st::int_overflow, "Overflowing page length"};
+
+	long pageno = ogg_page_pageno(&page);
+	if (pageno != next_page_no)
+		fprintf(stderr, "Output page number mismatch: expected %ld, got %ld.\n", next_page_no, pageno);
+	next_page_no = pageno + 1;
+
 	auto header_len = static_cast<size_t>(page.header_len);
 	auto body_len = static_cast<size_t>(page.body_len);
 	if (fwrite(page.header, 1, header_len, file) < header_len)
-		return {st::standard_error, "fwrite error: "s + strerror(errno)};
+		throw status {st::standard_error, "fwrite error: "s + strerror(errno)};
 	if (fwrite(page.body, 1, body_len, file) < body_len)
-		return {st::standard_error, "fwrite error: "s + strerror(errno)};
-	return st::ok;
+		throw status {st::standard_error, "fwrite error: "s + strerror(errno)};
 }
 
-ot::status ot::ogg_writer::write_header_packet(int serialno, int pageno, ogg_packet& packet)
+void 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"};
+		throw status {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 {ot::st::error,
-		        "Writing header packets spanning multiple pages are not yet supported. "
-		        "Please file an issue to make your wish known."};
+	while (ogg_stream_flush(&stream, &page) != 0)
+		write_page(page);
+
 	if (ogg_stream_check(&stream) != 0)
-		return {st::libogg_error, "ogg_stream_check failed"};
-	return ot::st::ok;
+		throw status {st::libogg_error, "ogg_stream_check failed"};
+}
+
+void ot::renumber_page(ogg_page& page, long new_pageno)
+{
+	// Quick optimization: don’t bother recomputing the CRC if the pageno did not change.
+	long old_pageno = ogg_page_pageno(&page);
+	if (old_pageno == new_pageno)
+		return;
+
+	/** The pageno field is located at bytes 18 to 21 (0-indexed, little-endian). */
+	uint32_t le_pageno = htole32(new_pageno);
+	memcpy(&page.header[18], &le_pageno, 4);
+	ogg_page_checksum_set(&page);
 }

src/opus.cc

@@ -3,7 +3,7 @@
  * \ingroup opus
  *
  * The way Opus is encapsulated into an Ogg stream, and the content of the packets we're dealing
- * with here is defined by [RFC 7584](https://tools.ietf.org/html/rfc7845.html).
+ * with here is defined by [RFC 7845](https://tools.ietf.org/html/rfc7845.html).
  *
  * Section 3 "Packet Organization" is critical for us:
  *
@@ -18,80 +18,74 @@
  *
  * \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.
  *
  */
 
 #include <opustags.h>
 
 #include <string.h>
+#include <algorithm>
 
-#ifdef __APPLE__
-#include <libkern/OSByteOrder.h>
-#define htole32(x) OSSwapHostToLittleInt32(x)
-#define le32toh(x) OSSwapLittleToHostInt32(x)
-#endif
-
-/**
- * \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)
+ot::opus_tags ot::parse_tags(const ogg_packet& packet)
 {
 	if (packet.bytes < 0)
-		return {st::int_overflow, "Overflowing comment header length"};
+		throw status {st::int_overflow, "Overflowing comment header length"};
 	size_t size = static_cast<size_t>(packet.bytes);
-	const char* data = reinterpret_cast<char*>(packet.packet);
+	const uint8_t* data = reinterpret_cast<uint8_t*>(packet.packet);
 	size_t pos = 0;
 	opus_tags my_tags;
 
 	// Magic number
 	if (8 > size)
-		return {st::cut_magic_number, "Comment header too short for the magic number"};
-	if (memcmp(data, "OpusTags", 8) != 0)
-		return {st::bad_magic_number, "Comment header did not start with OpusTags"};
+		throw status {st::cut_magic_number, "Comment header too short for the magic number"};
+	if (memcmp(data, u8"OpusTags", 8) != 0)
+		throw status {st::bad_magic_number, "Comment header did not start with OpusTags"};
 
 	// Vendor
 	pos = 8;
 	if (pos + 4 > size)
-		return {st::cut_vendor_length,
+		throw status {st::cut_vendor_length,
 		        "Vendor string length did not fit the comment header"};
 	size_t vendor_length = le32toh(*((uint32_t*) (data + pos)));
 	if (pos + 4 + vendor_length > size)
-		return {st::cut_vendor_data, "Vendor string did not fit the comment header"};
-	my_tags.vendor = std::string(data + pos + 4, vendor_length);
+		throw status {st::cut_vendor_data, "Vendor string did not fit the comment header"};
+	my_tags.vendor = std::u8string(reinterpret_cast<const char8_t*>(&data[pos + 4]), vendor_length);
 	pos += 4 + my_tags.vendor.size();
 
 	// Comment count
 	if (pos + 4 > size)
-		return {st::cut_comment_count, "Comment count did not fit the comment header"};
-	uint32_t count = le32toh(*((uint32_t*) (data + pos)));
+		throw status {st::cut_comment_count, "Comment count did not fit the comment header"};
+	uint32_t count;
+	memcpy(&count, data + pos, sizeof(count));
+	count = le32toh(count);
 	pos += 4;
 
 	// Comments' data
 	for (uint32_t i = 0; i < count; ++i) {
 		if (pos + 4 > size)
-			return {st::cut_comment_length,
-			        "Comment length did not fit the comment header"};
-		uint32_t comment_length = le32toh(*((uint32_t*) (data + pos)));
+			throw status {st::cut_comment_length,
+			              "Comment length did not fit the comment header"};
+		uint32_t comment_length;
+		memcpy(&comment_length, data + pos, sizeof(comment_length));
+		comment_length = le32toh(comment_length);
 		if (pos + 4 + comment_length > size)
-			return {st::cut_comment_data,
-			        "Comment string did not fit the comment header"};
-		const char *comment_value = data + pos + 4;
+			throw status {st::cut_comment_data,
+			              "Comment string did not fit the comment header"};
+		auto comment_value = reinterpret_cast<const char8_t*>(&data[pos + 4]);
 		my_tags.comments.emplace_back(comment_value, comment_length);
 		pos += 4 + comment_length;
 	}
 
 	// Extra data
-	my_tags.extra_data = std::string(data + pos, size - pos);
+	my_tags.extra_data = byte_string(data + pos, size - pos);
 
-	tags = std::move(my_tags);
-	return st::ok;
+	return my_tags;
 }
 
 ot::dynamic_ogg_packet ot::render_tags(const opus_tags& tags)
 {
 	size_t size = 8 + 4 + tags.vendor.size() + 4;
-	for (const std::string& comment : tags.comments)
+	for (const std::u8string& comment : tags.comments)
 		size += 4 + comment.size();
 	size += tags.extra_data.size();
 
@@ -111,7 +105,7 @@ ot::dynamic_ogg_packet ot::render_tags(const opus_tags& tags)
 	n = htole32(tags.comments.size());
 	memcpy(data, &n, 4);
 	data += 4;
-	for (const std::string& comment : tags.comments) {
+	for (const std::u8string& comment : tags.comments) {
 		n = htole32(comment.size());
 		memcpy(data, &n, 4);
 		memcpy(data+4, comment.data(), comment.size());
@@ -121,3 +115,107 @@ ot::dynamic_ogg_packet ot::render_tags(const opus_tags& tags)
 
 	return op;
 }
+
+/**
+ * The METADATA_BLOCK_PICTURE binary data, after base64 decoding, is organized like this:
+ *
+ *  - 4 bytes for the picture type,
+ *  - 4 + n bytes for the MIME type,
+ *  - 4 + n bytes for the description string,
+ *  - 16 bytes of picture attributes,
+ *  - 4 + n bytes for the picture data.
+ *
+ * Integers are all big endian.
+ */
+ot::picture::picture(ot::byte_string block)
+	: storage(std::move(block))
+{
+	size_t mime_offset = 4;
+	if (storage.size() < mime_offset + 4)
+		throw status { st::invalid_size, "missing MIME type in picture block" };
+	uint32_t mime_size = be32toh(*reinterpret_cast<const uint32_t*>(&storage[mime_offset]));
+
+	size_t desc_offset = mime_offset + 4 + mime_size;
+	if (storage.size() < desc_offset + 4)
+		throw status { st::invalid_size, "missing description in picture block" };
+	uint32_t desc_size;
+	memcpy(&desc_size, &storage[desc_offset], sizeof(desc_size));
+	desc_size = be32toh(desc_size);
+
+	size_t pic_offset = desc_offset + 4 + desc_size + 16;
+	if (storage.size() < pic_offset + 4)
+		throw status { st::invalid_size, "missing picture data in picture block" };
+	uint32_t pic_size;
+	memcpy(&pic_size, &storage[pic_offset], sizeof(pic_size));
+	pic_size = be32toh(pic_size);
+
+	if (storage.size() != pic_offset + 4 + pic_size)
+		throw status { st::invalid_size, "invalid picture block size" };
+
+	mime_type = byte_string_view(&storage[mime_offset + 4], mime_size);
+	picture_data = byte_string_view(&storage[pic_offset + 4], pic_size);
+}
+
+ot::byte_string ot::picture::serialize() const
+{
+	ot::byte_string bytes;
+	size_t mime_offset = 4;
+	size_t pic_offset = mime_offset + 4 + mime_type.size() + 4 + 0 + 16;
+	bytes.resize(pic_offset + 4 + picture_data.size());
+	*reinterpret_cast<uint32_t*>(&bytes[0]) = htobe32(3); // Picture type: front cover.
+	*reinterpret_cast<uint32_t*>(&bytes[mime_offset]) = htobe32(mime_type.size());
+	std::copy(mime_type.begin(), mime_type.end(), std::next(bytes.begin(), mime_offset + 4));
+	uint32_t picture_data_size = htobe32(picture_data.size());
+	memcpy(&bytes[pic_offset], &picture_data_size, sizeof(picture_data_size));
+	std::copy(picture_data.begin(), picture_data.end(), std::next(bytes.begin(), pic_offset + 4));
+	return bytes;
+}
+
+/**
+ * \todo Take into account the picture types (first 4 bytes of the tag value).
+ */
+std::optional<ot::picture> ot::extract_cover(const ot::opus_tags& tags)
+{
+	static const std::u8string_view prefix = u8"METADATA_BLOCK_PICTURE="sv;
+	auto is_cover = [](const std::u8string& tag) { return tag.starts_with(prefix); };
+	auto cover_tag = std::find_if(tags.comments.begin(), tags.comments.end(), is_cover);
+	if (cover_tag == tags.comments.end())
+		return {}; // No cover art.
+
+	auto extra_cover_tag = std::find_if(std::next(cover_tag), tags.comments.end(), is_cover);
+	if (extra_cover_tag != tags.comments.end())
+		fputs("warning: Found multiple covers; only the first will be extracted."
+		              " Please report your use case if you need a finer selection.\n", stderr);
+
+	std::u8string_view cover_value = *cover_tag;
+	cover_value.remove_prefix(prefix.size());
+	return picture(decode_base64(cover_value));
+}
+
+/**
+ * Detect the MIME type of the given data block by checking the first bytes. Only the most common
+ * image formats are currently supported. Using magic(5) would give better results but that level of
+ * exhaustiveness is probably not necessary.
+ */
+static ot::byte_string_view detect_mime_type(ot::byte_string_view data)
+{
+	static std::initializer_list<std::pair<ot::byte_string_view, ot::byte_string_view>> magic_numbers = {
+		{ "\xff\xd8\xff"_bsv, "image/jpeg"_bsv },
+		{ "\x89PNG"_bsv, "image/png"_bsv },
+		{ "GIF8"_bsv, "image/gif"_bsv },
+	};
+	for (auto [magic, mime] : magic_numbers) {
+		if (data.starts_with(magic))
+			return mime;
+	}
+	fputs("warning: Could not identify the MIME type of the picture; defaulting to application/octet-stream.\n", stderr);
+	return "application/octet-stream"_bsv;
+}
+
+std::u8string ot::make_cover(ot::byte_string_view picture_data)
+{
+	picture pic;
+	pic.mime_type = detect_mime_type(picture_data);
+	pic.picture_data = picture_data;
+	return u8"METADATA_BLOCK_PICTURE=" + encode_base64(pic.serialize());
+}

src/opustags.cc

@@ -15,17 +15,14 @@
  * Does practically nothing but call the cli module.
  */
 int main(int argc, char** argv) {
-	setlocale(LC_ALL, "");
-	ot::options opt;
-	ot::status rc = ot::parse_options(argc, argv, opt);
-	if (rc == ot::st::ok)
-		rc = ot::run(opt);
-
-	if (rc != ot::st::ok) {
+	try {
+		setlocale(LC_ALL, "");
+		ot::options opt = ot::parse_options(argc, argv, stdin);
+		ot::run(opt);
+		return 0;
+	} catch (const ot::status& rc) {
 		if (!rc.message.empty())
 			fprintf(stderr, "error: %s\n", rc.message.c_str());
-		return EXIT_FAILURE;
-	} else {
-		return EXIT_SUCCESS;
+		return rc == ot::st::bad_arguments ? 2 : 1;
 	}
 }

src/opustags.h

@@ -24,26 +24,45 @@
 
 #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>
 
+#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)
+#define htobe32(x) OSSwapHostToBigInt32(x)
+#define be32toh(x) OSSwapBigToHostInt32(x)
+#endif
+
+using namespace std::literals;
+
 namespace ot {
 
 /**
  * Possible return status code, ranging from errors to special statuses. They are usually
  * accompanied with a message with the #status structure.
  *
- * Functions that return non-ok status codes to signal special conditions like #end_of_stream should
- * 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.
  *
@@ -57,12 +76,12 @@ enum class st {
 	error,
 	standard_error, /**< Error raised by the C standard library. */
 	int_overflow,
+	cancel,
 	/* System */
 	badly_encoded,
-	information_lost,
+	child_process_failed,
 	/* Ogg */
 	bad_stream,
-	end_of_stream,
 	libogg_error,
 	/* Opus */
 	bad_magic_number,
@@ -72,41 +91,44 @@ enum class st {
 	cut_comment_count,
 	cut_comment_length,
 	cut_comment_data,
+	invalid_size,
 	/* CLI */
 	bad_arguments,
 };
 
 /**
  * Wraps a status code with an optional message. It is implictly converted to and from a
- * #status_code.
+ * #status_code. It may be thrown on error by any of the ot:: functions.
  *
  * 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) {}
 	template<class T> status(st code, T&& message) : code(code), message(message) {}
-	operator st() { return code; }
+	operator st() const { return code; }
 	st code;
 	std::string message;
 };
 
+using byte_string = std::basic_string<uint8_t>;
+using byte_string_view = std::basic_string_view<uint8_t>;
+
 /***********************************************************************************************//**
  * \defgroup system System
  * \{
  */
 
+/** fclose wrapper for std::unique_ptr’s deleter. */
+void close_file(FILE*);
+
 /**
  * Smart auto-closing FILE* handle.
  *
  * It implictly converts from an already opened FILE*.
  */
-struct file : std::unique_ptr<FILE, decltype(&fclose)> {
-	file(FILE* f = nullptr) : std::unique_ptr<FILE, decltype(&fclose)>(f, &fclose) {}
+struct file : std::unique_ptr<FILE, decltype(&close_file)> {
+	file(FILE* f = nullptr) : std::unique_ptr<FILE, decltype(&close_file)>(f, &close_file) {}
 };
 
 /**
@@ -123,9 +145,9 @@ public:
 	 * 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);
+	void open(const char* destination);
 	/** Close then move the partial file to its final location. */
-	ot::status commit();
+	void commit();
 	/** Delete the temporary file. */
 	void abort();
 	/** Get the underlying FILE* handle. */
@@ -138,28 +160,35 @@ private:
 	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. If some character could not be converted perfectly, keep converting
-	 * the string and finally return #st::information_lost.
-	 */
-	status operator()(const std::string& in, std::string& out)
-		{ return (*this)(in.data(), in.size(), out); }
-	status operator()(const char* in, size_t n, std::string& out);
-private:
-	iconv_t cd; /**< conversion descriptor */
-};
+/** Read a whole file into memory and return the read content. */
+byte_string slurp_binary_file(const char* filename);
+
+/** Convert a string from the system locale’s encoding to UTF-8. */
+std::u8string encode_utf8(std::string_view);
+
+/** Convert a string from UTF-8 to the system locale’s encoding. */
+std::string decode_utf8(std::u8string_view);
+
+/** 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.
+ */
+void run_editor(std::string_view editor, std::string_view path);
+
+/**
+ * Return the specified path’s mtime, i.e. the last data modification
+ * timestamp.
+ */
+timespec get_file_timestamp(const char* path);
+
+std::u8string encode_base64(byte_string_view src);
+byte_string decode_base64(std::u8string_view src);
 
 /** \} */
 
@@ -193,12 +222,8 @@ 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.
- *
- * \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.
+ * Call #read_page repeatedly until it returns false to consume the stream, and use #page to check
+ * its content.
  */
 struct ogg_reader {
 	/**
@@ -214,13 +239,12 @@ struct 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
-	 * next call to #read_page.
+	 * Read the next page from the input file. The result is made available in the #page field,
+	 * is owned by the Ogg reader, and is valid until the next call to #read_page.
 	 *
-	 * After the last page was read, return #status::end_of_stream.
+	 * Return true if a page was read, false on end of stream.
 	 */
-	status next_page();
+	bool next_page();
 	/**
 	 * 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
@@ -229,7 +253,7 @@ struct ogg_reader {
 	 * It is currently limited to packets that fit on a single page, and should be later
 	 * extended to support packets spanning multiple pages.
 	 */
-	status process_header_packet(const std::function<status(ogg_packet&)>& f);
+	void process_header_packet(const std::function<void(ogg_packet&)>& f);
 	/**
 	 * Current page from the sync state.
 	 *
@@ -237,6 +261,12 @@ struct ogg_reader {
 	 * to ogg_sync_pageout, wrapped by #read_page.
 	 */
 	ogg_page page;
+	/**
+	 * Page number in the physical stream of the last read page, disregarding multiplexed
+	 * streams. The first page number is 0. When no page has been read, its value is
+	 * (size_t) -1.
+	 */
+	size_t absolute_page_no = -1;
 	/**
 	 * The file is our source of binary data. It is not integrated to libogg, so we need to
 	 * handle it ourselves.
@@ -271,17 +301,26 @@ struct ogg_writer {
 	 *
 	 * This is a basic I/O operation and does not even require libogg, or the stream.
 	 */
-	status write_page(const ogg_page& page);
+	void write_page(const ogg_page& page);
 	/**
 	 * Write a header packet and flush the page. Header packets are always placed alone on their
 	 * pages.
 	 */
-	status write_header_packet(int serialno, int pageno, ogg_packet& packet);
+	void 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;
+	/**
+	 * Custom counter for the sequential page number to be written. It allows us to detect
+	 * ogg_page_pageno mismatches and renumber the pages if needed.
+	 */
+	long next_page_no = 0;
 };
 
 /**
@@ -301,6 +340,9 @@ private:
 	std::unique_ptr<unsigned char[]> data;
 };
 
+/** Update the Ogg pageno field in the given page. The CRC is recomputed if needed. */
+void renumber_page(ogg_page& page, long new_pageno);
+
 /** \} */
 
 /***********************************************************************************************//**
@@ -320,7 +362,7 @@ struct opus_tags {
 	 * OpusTags packets begin with a vendor string, meant to identify the implementation of the
 	 * encoder. It is expected to be an arbitrary UTF-8 string.
 	 */
-	std::string vendor;
+	std::u8string vendor;
 	/**
 	 * Comments are strings in the NAME=Value format. A comment may also be called a field, or a
 	 * tag.
@@ -329,7 +371,7 @@ struct opus_tags {
 	 * 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;
+	std::list<std::u8string> comments;
 	/**
 	 * According to RFC 7845:
 	 * > Immediately following the user comment list, the comment header MAY contain
@@ -341,21 +383,54 @@ struct opus_tags {
 	 * In the future, we could add options to manipulate this data: view it, edit it, truncate
 	 * it if it's marked as padding, truncate it unconditionally.
 	 */
-	std::string extra_data;
+	byte_string extra_data;
 };
 
 /**
  * Read the given OpusTags packet and extract its content into an opus_tags object.
- *
- * On error, the tags object is left unchanged.
  */
-status parse_tags(const ogg_packet& packet, opus_tags& tags);
+opus_tags parse_tags(const ogg_packet& packet);
 
 /**
  * Serialize an #opus_tags object into an OpusTags Ogg packet.
  */
 dynamic_ogg_packet render_tags(const opus_tags& tags);
 
+/**
+ * Extracted data from the METADATA_BLOCK_PICTURE tag. See
+ * <https://xiph.org/flac/format.html#metadata_block_picture> for the full specifications.
+ *
+ * It may contain all kinds of metadata but most are not used at all. For now, let’s assume all
+ * pictures have picture type 3 (front cover), and empty metadata.
+ */
+struct picture {
+	picture() = default;
+
+	/** Extract the picture information from serialized binary data.*/
+	picture(byte_string block);
+	byte_string_view mime_type;
+	byte_string_view picture_data;
+
+	/**
+	 * Encode the picture attributes (mime_type, picture_data) into a binary block to be stored
+	 * into METADATA_BLOCK_PICTURE.
+	 */
+	byte_string serialize() const;
+
+	/** To avoid needless copies of the picture data, move the original data block there. The
+	 *  string_view attributes will refer to it. */
+	byte_string storage;
+};
+
+/** Extract the first picture embedded in the tags, regardless of its type. */
+std::optional<picture> extract_cover(const opus_tags& tags);
+
+/**
+ * Return a METADATA_BLOCK_PICTURE tag defining the front cover art to the given picture data (JPEG,
+ * PNG). The MIME type is deduced from the magic number.
+ */
+std::u8string make_cover(byte_string_view picture_data);
+
 /** \} */
 
 /***********************************************************************************************//**
@@ -374,18 +449,20 @@ struct options {
 	 */
 	bool print_help = false;
 	/**
-	 * Path to the input file. It cannot be empty. The special "-" string means stdin.
+	 * Paths to the input files. The special string "-" means stdin.
 	 *
-	 * This is the mandatory non-flagged parameter.
+	 * At least one input file must be given. If `--in-place` is used,
+	 * more than one may be given.
 	 */
-	std::string path_in;
+	std::vector<std::string> paths_in;
 	/**
-	 * Path to the optional file. The special "-" string means stdout. When empty, opustags runs
-	 * in read-only mode. For in-place editing, path_out is defined equal to path_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::string path_out;
+	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.
@@ -393,6 +470,21 @@ struct options {
 	 * 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.
@@ -402,76 +494,97 @@ struct options {
 	 * #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::vector<std::string> to_delete;
+	std::list<std::u8string> to_delete;
 	/**
 	 * Delete all the existing comments.
 	 *
-	 * Option: --delete-all
+	 * 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::vector<std::string> to_add;
+	std::list<std::u8string> to_add;
 	/**
-	 * Replace the previous comments by the ones supplied by the user.
+	 * If set, the input file’s cover art is exported to the specified file. - for stdout. Does
+	 * not overwrite the file if it already exists unless -y is specified. Does nothing if the
+	 * input file does not contain a cover art.
 	 *
-	 * Read a list of comments from stdin and populate #to_add. Further comments may be added
-	 * with the --add option.
-	 *
-	 * Option: --set-all
+	 * Option: --output-cover
 	 */
-	bool set_all = false;
+	std::optional<std::string> cover_out;
+	/**
+	 * Print the vendor string at the beginning of the OpusTags packet instead of printing the
+	 * tags. Only applicable in read-only mode.
+	 *
+	 * Option: --vendor
+	 */
+	bool print_vendor = false;
+	/**
+	 * Replace the vendor string by the one specified by the user.
+	 *
+	 * Option: --set-vendor
+	 */
+	std::optional<std::u8string> set_vendor;
+	/**
+	 * 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;
+	/**
+	 * In text mode (default), tags are separated by a line feed. However, when combining
+	 * opustags with grep or other line-based tools, this proves to be a bad separator because
+	 * tag values may contain newlines. Changing the delimiter to '\0' with -z eases the
+	 * processing of multi-line tags with other tools that support null-terminated lines.
+	 */
+	char tag_delimiter = '\n';
 };
 
 /**
  * Parse the command-line arguments. Does not perform I/O related validations, but checks the
- * consistency of its arguments.
- *
- * On error, the state of the options structure is unspecified.
+ * consistency of its arguments. Comments are read if necessary from the given stream.
  */
-status parse_options(int argc, char** argv, options& opt);
+options parse_options(int argc, char** argv, 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.
  *
- * The comments must be encoded in UTF-8, and are converted to the system locale when printed.
+ * The comments must be encoded in UTF-8, and are converted to the system locale when printed,
+ * unless raw is true.
  *
- * The output generated is meant to be parseable by #ot::read_tags.
+ * The output generated is meant to be parseable by #ot::read_comments.
  */
-void print_comments(const std::list<std::string>& comments, FILE* output);
+void print_comments(const std::list<std::u8string>& comments, FILE* output, const options& opt);
 
 /**
- * Parse the comments outputted by #ot::print_comments.
- *
- * The comments are converted from the system encoding to UTF-8, and returned as UTF-8.
+ * Parse the comments outputted by #ot::print_comments. Unless raw is true, the comments are
+ * converted from the system encoding to UTF-8, and returned as UTF-8.
  */
-status read_comments(FILE* input, std::list<std::string>& comments);
+std::list<std::u8string> read_comments(FILE* input, const options& opt);
 
 /**
  * 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 strings are all UTF-8.
  */
-void delete_comments(std::list<std::string>& comments, const std::string& selector);
+void delete_comments(std::list<std::u8string>& comments, const std::u8string& selector);
 
 /**
  * Main entry point to the opustags program, and pretty much the same as calling opustags from the
  * command-line.
  */
-status run(const options& opt);
+void run(const options& opt);
 
 /** \} */
 
 }
+
+/** Handy literal suffix for building byte strings. */
+ot::byte_string operator""_bs(const char* data, size_t size);
+ot::byte_string_view operator""_bsv(const char* data, size_t size);

src/system.cc

@@ -12,37 +12,87 @@
 #include <opustags.h>
 
 #include <errno.h>
+#include <fstream>
+#include <stdlib.h>
 #include <string.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
-ot::status ot::partial_file::open(const char* destination)
+ot::byte_string operator""_bs(const char* data, size_t size)
+{
+	return ot::byte_string(reinterpret_cast<const uint8_t*>(data), size);
+}
+
+ot::byte_string_view operator""_bsv(const char* data, size_t size)
+{
+	return ot::byte_string_view(reinterpret_cast<const uint8_t*>(data), size);
+}
+
+void ot::close_file(FILE* file)
+{
+	fclose(file);
+}
+
+void 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)};
+		throw status {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;
+		throw status {st::standard_error,
+		              "Could not get the partial file handle to '" + temporary_name + "': " +
+		              strerror(errno)};
 }
 
-ot::status ot::partial_file::commit()
+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));
+}
+
+void ot::partial_file::commit()
 {
 	if (file == nullptr)
-		return st::ok;
+		return;
 	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;
+		throw status {st::standard_error,
+		              "Could not move the result file '" + temporary_name + "' to '" +
+		              final_name + "': " + strerror(errno) + "."};
 }
 
 void ot::partial_file::abort()
@@ -53,47 +103,178 @@ void ot::partial_file::abort()
 	remove(temporary_name.c_str());
 }
 
-ot::encoding_converter::encoding_converter(const char* from, const char* to)
+/**
+ * Determine the file size, in bytes, of the given file. Return -1 on for streams.
+ */
+static long get_file_size(FILE* f)
+{
+	if (fseek(f, 0L, SEEK_END) != 0) {
+		clearerr(f); // Recover.
+		return -1;
+	}
+	long file_size = ftell(f);
+	rewind(f);
+	return file_size;
+}
+
+ot::byte_string ot::slurp_binary_file(const char* filename)
+{
+	file f = strcmp(filename, "-") == 0 ? freopen(nullptr, "rb", stdin)
+	                                    : fopen(filename, "rb");
+	if (f == nullptr)
+		throw status { st::standard_error,
+		               "Could not open '"s + filename + "': " + strerror(errno) + "." };
+
+	byte_string content;
+	long file_size = get_file_size(f.get());
+	if (file_size < 0) {
+		// Read the input stream block by block and resize the output byte string as needed.
+		uint8_t buffer[4096];
+		while (!feof(f.get())) {
+			size_t read_len = fread(buffer, 1, sizeof(buffer), f.get());
+			content.append(buffer, read_len);
+			if (ferror(f.get()))
+				throw status { st::standard_error,
+					       "Could not read '"s + filename + "': " + strerror(errno) + "." };
+		}
+	} else {
+		// Lucky! We know the file size, so let’s slurp it at once.
+		content.resize(file_size);
+		if (fread(content.data(), 1, file_size, f.get()) < size_t(file_size))
+			throw status { st::standard_error,
+				       "Could not read '"s + filename + "': " + strerror(errno) + "." };
+	}
+
+	return content;
+}
+
+/** 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.
+	 */
+	template<class InChar, class OutChar>
+	std::basic_string<OutChar> convert(std::basic_string_view<InChar>);
+private:
+	iconv_t cd; /**< conversion descriptor */
+};
+
+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()
+encoding_converter::~encoding_converter()
 {
 	iconv_close(cd);
 }
 
-ot::status ot::encoding_converter::operator()(const char* in, size_t n, std::string& out)
+template<class InChar, class OutChar>
+std::basic_string<OutChar> encoding_converter::convert(std::basic_string_view<InChar> in)
 {
 	iconv(cd, nullptr, nullptr, nullptr, nullptr);
-	out.clear();
-	out.reserve(n);
-	char* in_cursor = const_cast<char*>(in);
-	size_t in_left = n;
+	std::basic_string<OutChar> out;
+	out.reserve(in.size());
+	const char* in_data = reinterpret_cast<const char*>(in.data());
+	char* in_cursor = const_cast<char*>(in_data);
+	size_t in_left = in.size();
 	constexpr size_t chunk_size = 1024;
 	char chunk[chunk_size];
-	bool lost_information = false;
 	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)
-			return {ot::st::badly_encoded,
-			        "Could not convert string '" + std::string(in, n) + "': " +
-			        strerror(errno)};
-		if (rc != 0)
-			lost_information = true;
-		out.append(chunk, out_cursor - chunk);
+
+		if (rc == (size_t) -1 && errno == E2BIG) {
+			// Loop normally.
+		} else if (rc == (size_t) -1) {
+			throw ot::status {ot::st::badly_encoded, strerror(errno) + "."s};
+		} else if (rc != 0) {
+			throw ot::status {ot::st::badly_encoded,
+			                 "Some characters could not be converted into the target encoding."};
+		}
+
+		out.append(reinterpret_cast<OutChar*>(chunk), out_cursor - chunk);
 		if (in_cursor == nullptr)
 			break;
 		else if (in_left == 0)
 			in_cursor = nullptr;
 	}
-	if (lost_information)
-		return {ot::st::information_lost,
-		        "Some characters could not be converted into the target encoding "
-		        "in string '" + std::string(in, n) + "'."};
-	return ot::st::ok;
+	return out;
+}
+
+std::u8string ot::encode_utf8(std::string_view in)
+{
+	static encoding_converter to_utf8_cvt("", "UTF-8");
+	return to_utf8_cvt.convert<char, char8_t>(in);
+}
+
+std::string ot::decode_utf8(std::u8string_view in)
+{
+	static encoding_converter from_utf8_cvt("UTF-8", "");
+	return from_utf8_cvt.convert<char8_t, char>(in);
+}
+
+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;
+}
+
+void 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)
+		throw ot::status {st::standard_error, "waitpid error: "s + strerror(errno)};
+	else if (!WIFEXITED(status))
+		throw ot::status {st::child_process_failed,
+		                 "Child process did not terminate normally: "s + strerror(errno)};
+	else if (WEXITSTATUS(status) != 0)
+		throw ot::status {st::child_process_failed,
+		                  "Child process exited with " + std::to_string(WEXITSTATUS(status))};
+}
+
+timespec ot::get_file_timestamp(const char* path)
+{
+	timespec mtime;
+	struct stat st;
+	if (stat(path, &st) == -1)
+		throw status {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 mtime;
 }

t/CMakeLists.txt

@@ -10,13 +10,17 @@ target_link_libraries(ogg.t ot)
 add_executable(cli.t EXCLUDE_FROM_ALL cli.cc)
 target_link_libraries(cli.t ot)
 
+add_executable(base64.t EXCLUDE_FROM_ALL base64.cc)
+target_link_libraries(base64.t ot)
+
 add_executable(oggdump EXCLUDE_FROM_ALL oggdump.cc)
 target_link_libraries(oggdump ot)
 
 configure_file(gobble.opus . COPYONLY)
+configure_file(pixel.png . COPYONLY)
 
 add_custom_target(
 	check
 	COMMAND prove "${CMAKE_CURRENT_BINARY_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}"
-	DEPENDS opustags gobble.opus system.t opus.t ogg.t cli.t
+	DEPENDS opustags gobble.opus system.t opus.t ogg.t cli.t base64.t
 )

t/base64.cc

@@ -0,0 +1,46 @@
+#include <opustags.h>
+#include "tap.h"
+
+static void check_encode_base64()
+{
+	opaque_is(ot::encode_base64(""_bsv), u8"", "empty");
+	opaque_is(ot::encode_base64("a"_bsv), u8"YQ==", "1 character");
+	opaque_is(ot::encode_base64("aa"_bsv), u8"YWE=", "2 characters");
+	opaque_is(ot::encode_base64("aaa"_bsv), u8"YWFh", "3 characters");
+	opaque_is(ot::encode_base64("aaaa"_bsv), u8"YWFhYQ==", "4 characters");
+	opaque_is(ot::encode_base64("\xFF\xFF\xFE"_bsv), u8"///+", "RFC alphabet");
+	opaque_is(ot::encode_base64("\0x"_bsv), u8"AHg=", "embedded null bytes");
+}
+
+static void check_decode_base64()
+{
+	opaque_is(ot::decode_base64(u8""), ""_bsv, "empty");
+	opaque_is(ot::decode_base64(u8"YQ=="), "a"_bsv, "1 character");
+	opaque_is(ot::decode_base64(u8"YWE="), "aa"_bsv, "2 characters");
+	opaque_is(ot::decode_base64(u8"YQ"), "a"_bsv, "padless 1 character");
+	opaque_is(ot::decode_base64(u8"YWE"), "aa"_bsv, "padless 2 characters");
+	opaque_is(ot::decode_base64(u8"YWFh"), "aaa"_bsv, "3 characters");
+	opaque_is(ot::decode_base64(u8"YWFhYQ=="), "aaaa"_bsv, "4 characters");
+	opaque_is(ot::decode_base64(u8"///+"), "\xFF\xFF\xFE"_bsv, "RFC alphabet");
+	opaque_is(ot::decode_base64(u8"AHg="), "\0x"_bsv, "embedded null bytes");
+
+	try {
+		ot::decode_base64(u8"Y===");
+		throw failure("accepted a bad block size");
+	} catch (const ot::status& e) {
+	}
+
+	try {
+		ot::decode_base64(u8"\xFF bad message!");
+		throw failure("accepted an invalid character");
+	} catch (const ot::status& e) {
+	}
+}
+
+int main(int argc, char **argv)
+{
+	std::cout << "1..2\n";
+	run(check_encode_base64, "base64 encoding");
+	run(check_decode_base64, "base64 decoding");
+	return 0;
+}

t/cli.cc

@@ -3,50 +3,90 @@
 
 #include <string.h>
 
-using namespace std::literals::string_literals;
+static ot::status read_comments(FILE* input, std::list<std::u8string>& comments, bool raw)
+{
+	ot::options opt;
+	opt.raw = raw;
+	try {
+		comments = ot::read_comments(input, opt);
+	} catch (const ot::status& rc) {
+		return rc;
+	}
+	return ot::st::ok;
+}
 
 void check_read_comments()
 {
-	std::list<std::string> comments;
+	std::list<std::u8string> 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);
+		rc = 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"};
+		auto&& expected = {u8"TITLE=a b c", u8"ARTIST=X", u8"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);
+		rc = 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 = read_comments(input.get(), comments, true);
+		if (rc != ot::st::ok)
+			throw failure("could not read comments");
+		if (comments.front() != (char8_t*) "RAW=\xFF\xFF")
+			throw failure("parsed user comments did not match expectations");
+	}
+	{
+		std::string txt = "MULTILINE=First\n\tSecond\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		rc = read_comments(input.get(), comments, true);
+		if (rc != ot::st::ok)
+			throw failure("could not read comments");
+		if (comments.front() != u8"MULTILINE=First\nSecond")
+			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);
+		rc = read_comments(input.get(), comments, false);
 		if (rc != ot::st::error)
 			throw failure("did not get the expected error reading malformed comments");
 	}
+	{
+		std::string txt = "\tBad"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		rc = read_comments(input.get(), comments, true);
+		if (rc != ot::st::error)
+			throw failure("did not get the expected error reading bad continuation line");
+	}
 }
 
 /**
  * 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)
+static ot::status parse_options(const std::vector<const char*>& args, ot::options& opt, FILE *comments)
 {
 	int argc = args.size();
 	char* argv[argc];
-	for (size_t i = 0; i < argc; ++i)
+	for (int i = 0; i < argc; ++i)
 		argv[i] = strdup(args[i]);
-	ot::status rc = ot::parse_options(argc, argv, opt);
-	for (size_t i = 0; i < argc; ++i)
+	ot::status rc = ot::st::ok;
+	try {
+		opt = ot::parse_options(argc, argv, comments);
+	} catch (const ot::status& e) {
+		rc = e;
+	}
+	for (int i = 0; i < argc; ++i)
 		free(argv[i]);
 	return rc;
 }
@@ -55,7 +95,9 @@ void check_good_arguments()
 {
 	auto parse = [](std::vector<const char*> args) {
 		ot::options opt;
-		ot::status rc = parse_options(args, 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;
@@ -67,66 +109,116 @@ void check_good_arguments()
 		throw failure("did not catch --help");
 
 	opt = parse({"opustags", "x", "--output", "y", "-D", "-s", "X=Y Z", "-d", "a=b"});
-	if (opt.path_in != "x" || opt.path_out != "y" || !opt.delete_all || opt.overwrite ||
-	    opt.to_delete.size() != 2 || opt.to_delete[0] != "X" || opt.to_delete[1] != "a=b" ||
-	    opt.to_add.size() != 1 || opt.to_add[0] != "X=Y Z")
+	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() != u8"X" || *std::next(opt.to_delete.begin()) != u8"a=b" ||
+	    opt.to_add != std::list<std::u8string>{ u8"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.path_in != "x" || opt.path_out != "x" || !opt.set_all || !opt.overwrite ||
-	    opt.to_delete.size() != 0 || opt.to_add.size() != 1 || opt.to_add[0] != "x=y z")
+	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::u8string>{ u8"N=1", u8"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() != u8"X=\xFF")
+		throw failure("--raw did not disable transcoding");
 }
 
 void check_bad_arguments()
 {
-	auto error_case = [](std::vector<const char*> args, const char* message, const std::string& name) {
+	auto error_code_case = [](std::vector<const char*> args, const char* message, ot::st error_code, const std::string& name) {
 		ot::options opt;
-		ot::status rc = parse_options(args, opt);
-		if (rc.code != ot::st::bad_arguments)
+		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)
+		if (!rc.message.starts_with(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", "--output", ""}, "Output file path cannot be empty.", "empty output path");
 	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", ""}, "Input file path cannot be empty.", "empty input file path");
 	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", "-S", "-"}, "Cannot use standard input more than once.", "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", "--output-cover", "x", "--output-cover", "y"},
+	           "Cannot specify --output-cover more than once.", "multiple --output-cover");
+	error_case({"opustags", "x", "-o", "-", "--output-cover", "-"},
+	            "Cannot specify standard output for both --output and --output-cover.", "-o and --output-cover conflict");
+	error_case({"opustags", "-i", "x", "y", "--output-cover", "z"},
+	            "Cannot use --output-cover with multiple input files.", "--output-cover with multiple input");
+	error_case({"opustags", "-i", "--vendor", "x"},
+	            "--vendor is only supported in read-only mode.", "--vendor when editing");
+	error_case({"opustags", "-d", "\xFF", "x"},
+	           "Could not encode argument into UTF-8:",
+	           "-d with binary data");
+	error_case({"opustags", "-a", "X=\xFF", "x"},
+	           "Could not encode argument into UTF-8:",
+	           "-a with binary data");
+	error_case({"opustags", "-s", "X=\xFF", "x"},
+	           "Could not encode argument into UTF-8:",
+	           "-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"};
+	using C = std::list<std::u8string>;
+	C original = {u8"TITLE=X", u8"Title=Y", u8"Title=Z", u8"ARTIST=A", u8"artIst=B"};
 
 	C edited = original;
-	ot::delete_comments(edited, "derp");
+	ot::delete_comments(edited, u8"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"};
+	ot::delete_comments(edited, u8"Title");
+	C expected = {u8"ARTIST=A", u8"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"};
+	ot::delete_comments(edited, u8"titlE=Y");
+	ot::delete_comments(edited, u8"Title=z");
+	expected = {u8"TITLE=X", u8"Title=Z", u8"ARTIST=A", u8"artIst=B"};
 	if (!std::equal(edited.begin(), edited.end(), expected.begin(), expected.end()))
 		throw failure("did not delete a specific title correctly");
 }

t/ogg.cc

@@ -11,37 +11,27 @@ static void check_ref_ogg()
 
 	ot::ogg_reader reader(input.get());
 
-	ot::status rc = reader.next_page();
-	if (rc != ot::st::ok)
+	if (reader.next_page() != true)
 		throw failure("could not read the first page");
 	if (!ot::is_opus_stream(reader.page))
 		throw failure("failed to identify the stream as opus");
-	rc = reader.process_header_packet([](ogg_packet& p) {
+	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");
 
-	rc = reader.next_page();
-	if (rc != ot::st::ok)
+	if (reader.next_page() != true)
 		throw failure("could not read the second page");
-	rc = reader.process_header_packet([](ogg_packet& p) {
+	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");
 
 	while (!ogg_page_eos(&reader.page)) {
-		rc = reader.next_page();
-		if (rc != ot::st::ok)
+		if (reader.next_page() != true)
 			throw failure("failure reading a page");
 	}
-	rc = reader.next_page();
-	if (rc != ot::st::end_of_stream)
+	if (reader.next_page() != false)
 		throw failure("did not correctly detect the end of stream");
 }
 
@@ -67,7 +57,6 @@ static void check_memory_ogg()
 	ogg_packet second_packet = make_packet("Second");
 	std::vector<unsigned char> my_ogg(128);
 	size_t my_ogg_size;
-	ot::status rc;
 
 	{
 		ot::file output = fmemopen(my_ogg.data(), my_ogg.size(), "w");
@@ -75,11 +64,7 @@ static void check_memory_ogg()
 			throw failure("could not open the output stream");
 		ot::ogg_writer writer(output.get());
 		writer.write_header_packet(1234, 0, first_packet);
-		if (rc != ot::st::ok)
-			throw failure("could not write the first packet");
 		writer.write_header_packet(1234, 1, second_packet);
-		if (rc != ot::st::ok)
-			throw failure("could not write the second packet");
 		my_ogg_size = ftell(output.get());
 		if (my_ogg_size != 67)
 			throw failure("unexpected output size");
@@ -90,28 +75,19 @@ static void check_memory_ogg()
 		if (input == nullptr)
 			throw failure("could not open the input stream");
 		ot::ogg_reader reader(input.get());
-		rc = reader.next_page();
-		if (rc != ot::st::ok)
+		if (reader.next_page() != true)
 			throw failure("could not read the first page");
-		rc = reader.process_header_packet([&first_packet](ogg_packet &p) {
+		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");
-		rc = reader.next_page();
-		if (rc != ot::st::ok)
+		if (reader.next_page() != true)
 			throw failure("could not read the second page");
-		rc = reader.process_header_packet([&second_packet](ogg_packet &p) {
+		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");
-		rc = reader.next_page();
-		if (rc != ot::st::end_of_stream)
+		if (reader.next_page() != false)
 			throw failure("unexpected third page");
 	}
 }
@@ -121,9 +97,13 @@ 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);
+	try {
+		reader.next_page();
+		throw failure("did not raise an error");
+	} catch (const ot::status& rc) {
+		if (rc != ot::st::bad_stream)
+			throw failure(err_msg);
+	}
 }
 
 void check_identification()
@@ -159,12 +139,29 @@ void check_identification()
 		throw failure("was not the beginning of a stream");
 }
 
+void check_renumber_page()
+{
+	ot::file input = fopen("gobble.opus", "r");
+	if (input == nullptr)
+		throw failure("could not open gobble.opus");
+
+	ot::ogg_reader reader(input.get());
+	if (reader.next_page() != true)
+		throw failure("could not read the first page");
+
+	long new_pageno = 1234;
+	ot::renumber_page(reader.page, new_pageno);
+	if (ogg_page_pageno(&reader.page) != new_pageno)
+		throw failure("renumbering failed");
+}
+
 int main(int argc, char **argv)
 {
-	std::cout << "1..4\n";
+	std::cout << "1..5\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");
+	run(check_renumber_page, "page renumbering");
 	return 0;
 }

t/opus.cc

@@ -3,8 +3,6 @@
 
 #include <string.h>
 
-using namespace std::literals::string_literals;
-
 static const char standard_OpusTags[] =
 	"OpusTags"
 	"\x14\x00\x00\x00" "opustags test packet"
@@ -14,27 +12,34 @@ static const char standard_OpusTags[] =
 
 static void parse_standard()
 {
-	ot::opus_tags tags;
 	ogg_packet op;
 	op.bytes = sizeof(standard_OpusTags) - 1;
 	op.packet = (unsigned char*) standard_OpusTags;
-	auto rc = ot::parse_tags(op, tags);
-	if (rc != ot::st::ok)
-		throw failure("ot::parse_tags did not return ok");
-	if (tags.vendor != "opustags test packet")
+	ot::opus_tags tags = ot::parse_tags(op);
+	if (tags.vendor != u8"opustags test packet")
 		throw failure("bad vendor string");
 	if (tags.comments.size() != 2)
 		throw failure("bad number of comments");
 	auto it = tags.comments.begin();
-	if (*it != "TITLE=Foo")
+	if (*it != u8"TITLE=Foo")
 		throw failure("bad title");
 	++it;
-	if (*it != "ARTIST=Bar")
+	if (*it != u8"ARTIST=Bar")
 		throw failure("bad artist");
 	if (tags.extra_data.size() != 0)
 		throw failure("found mysterious padding data");
 }
 
+static ot::status try_parse_tags(const ogg_packet& packet)
+{
+	try {
+		ot::parse_tags(packet);
+		return ot::st::ok;
+	} catch (const ot::status& rc) {
+		return rc;
+	}
+}
+
 /**
  * Try parse_tags with packets that should not valid, or that might even
  * corrupt the memory. Run this one with valgrind to ensure we're not
@@ -59,43 +64,40 @@ static void parse_corrupted()
 	char* end = packet + size;
 
 	op.bytes = 7;
-	if (ot::parse_tags(op, tags) != ot::st::cut_magic_number)
+	if (try_parse_tags(op) != ot::st::cut_magic_number)
 		throw failure("did not detect the overflowing magic number");
 	op.bytes = 11;
-	if (ot::parse_tags(op, tags) != ot::st::cut_vendor_length)
+	if (try_parse_tags(op) != ot::st::cut_vendor_length)
 		throw failure("did not detect the overflowing vendor string length");
 	op.bytes = size;
 
 	header_data[0] = 'o';
-	if (ot::parse_tags(op, tags) != ot::st::bad_magic_number)
+	if (try_parse_tags(op) != ot::st::bad_magic_number)
 		throw failure("did not detect the bad magic number");
 	header_data[0] = 'O';
 
 	*vendor_length = end - vendor_string + 1;
-	if (ot::parse_tags(op, tags) != ot::st::cut_vendor_data)
+	if (try_parse_tags(op) != ot::st::cut_vendor_data)
 		throw failure("did not detect the overflowing vendor string");
 	*vendor_length = end - vendor_string - 3;
-	if (ot::parse_tags(op, tags) != ot::st::cut_comment_count)
+	if (try_parse_tags(op) != ot::st::cut_comment_count)
 		throw failure("did not detect the overflowing comment count");
 	*vendor_length = comment_count - vendor_string;
 
 	++*comment_count;
-	if (ot::parse_tags(op, tags) != ot::st::cut_comment_length)
+	if (try_parse_tags(op) != ot::st::cut_comment_length)
 		throw failure("did not detect the overflowing comment length");
 	*first_comment_length = end - first_comment_data + 1;
-	if (ot::parse_tags(op, tags) != ot::st::cut_comment_data)
+	if (try_parse_tags(op) != ot::st::cut_comment_data)
 		throw failure("did not detect the overflowing comment data");
 }
 
 static void recode_standard()
 {
-	ot::opus_tags tags;
 	ogg_packet op;
 	op.bytes = sizeof(standard_OpusTags) - 1;
 	op.packet = (unsigned char*) standard_OpusTags;
-	auto rc = ot::parse_tags(op, tags);
-	if (rc != ot::st::ok)
-		throw failure("ot::parse_tags did not return ok");
+	ot::opus_tags tags = ot::parse_tags(op);
 	auto packet = ot::render_tags(tags);
 	if (packet.b_o_s != 0)
 		throw failure("b_o_s should not be set");
@@ -113,7 +115,6 @@ static void recode_standard()
 
 static void recode_padding()
 {
-	ot::opus_tags tags;
 	std::string padded_OpusTags(standard_OpusTags, sizeof(standard_OpusTags));
 	// ^ note: padded_OpusTags ends with a null byte here
 	padded_OpusTags += "hello";
@@ -121,10 +122,8 @@ static void recode_padding()
 	op.bytes = padded_OpusTags.size();
 	op.packet = (unsigned char*) padded_OpusTags.data();
 
-	auto rc = ot::parse_tags(op, tags);
-	if (rc != ot::st::ok)
-		throw failure("ot::parse_tags did not return ok");
-	if (tags.extra_data != "\0hello"s)
+	ot::opus_tags tags = ot::parse_tags(op);
+	if (tags.extra_data != "\0hello"_bsv)
 		throw failure("corrupted extra data");
 	// recode the packet and ensure it's exactly the same
 	auto packet = ot::render_tags(tags);
@@ -136,12 +135,60 @@ static void recode_padding()
 		throw failure("the rendered packet is not what we expected");
 }
 
+static void extract_cover()
+{
+	ot::byte_string_view picture_data = ""_bsv
+		"\x00\x00\x00\x03" // Picture type 3.
+		"\x00\x00\x00\x09" "image/foo" // MIME type.
+		"\x00\x00\x00\x00" "" // Description.
+		"\x00\x00\x00\x00" // Width.
+		"\x00\x00\x00\x00" // Height.
+		"\x00\x00\x00\x00" // Color depth.
+		"\x00\x00\x00\x00" // Palette size.
+		"\x00\x00\x00\x0C" "Picture data";
+
+	ot::opus_tags tags;
+	tags.comments = { u8"METADATA_BLOCK_PICTURE=" + ot::encode_base64(picture_data) };
+	std::optional<ot::picture> cover = ot::extract_cover(tags);
+	if (!cover)
+		throw failure("could not extract the cover");
+	if (cover->mime_type != "image/foo"_bsv)
+		throw failure("bad extracted MIME type");
+	if (cover->picture_data != "Picture data"_bsv)
+		throw failure("bad extracted picture data");
+
+	ot::byte_string_view truncated_data = picture_data.substr(0, picture_data.size() - 1);
+	tags.comments = { u8"METADATA_BLOCK_PICTURE=" + ot::encode_base64(truncated_data) };
+	try {
+		ot::extract_cover(tags);
+		throw failure("accepted a bad picture block");
+	} catch (const ot::status& rc) {}
+}
+
+static void make_cover()
+{
+	ot::byte_string_view picture_block = ""_bsv
+		"\x00\x00\x00\x03" // Picture type 3.
+		"\x00\x00\x00\x09" "image/png" // MIME type.
+		"\x00\x00\x00\x00" "" // Description.
+		"\x00\x00\x00\x00" // Width.
+		"\x00\x00\x00\x00" // Height.
+		"\x00\x00\x00\x00" // Color depth.
+		"\x00\x00\x00\x00" // Palette size.
+		"\x00\x00\x00\x11" "\x89PNG Picture data";
+
+	std::u8string expected = u8"METADATA_BLOCK_PICTURE=" + ot::encode_base64(picture_block);
+	opaque_is(ot::make_cover("\x89PNG Picture data"_bsv), expected, "build the picture tag");
+}
+
 int main()
 {
-	std::cout << "1..4\n";
+	std::cout << "1..6\n";
 	run(parse_standard, "parse a standard OpusTags packet");
 	run(parse_corrupted, "correctly reject invalid packets");
 	run(recode_standard, "recode a standard OpusTags packet");
 	run(recode_padding, "recode a OpusTags packet with padding");
+	run(extract_cover, "extract the cover art");
+	run(make_cover, "encode the cover art");
 	return 0;
 }

t/opustags.t

@@ -4,10 +4,12 @@ use strict;
 use warnings;
 use utf8;
 
-use Test::More tests => 34;
+use Test::More tests => 66;
+use Test::Deep qw(cmp_deeply re);
 
 use Digest::MD5;
 use File::Basename;
+use File::Copy;
 use IPC::Open3;
 use List::MoreUtils qw(any);
 use Symbol 'gensym';
@@ -43,7 +45,7 @@ sub opustags {
 # Tests related to the overall opustags executable, like the help message.
 # No Opus file is manipulated here.
 
-is_deeply(opustags(), ['', <<EOF, 256], 'no options is a failure');
+is_deeply(opustags(), ['', <<EOF, 512], 'no options is a failure');
 error: No arguments specified. Use -h for help.
 EOF
 
@@ -52,36 +54,16 @@ $help->[0] =~ /^([^\n]*+)/;
 my $version = $1;
 like($version, qr/^opustags version (\d+\.\d+\.\d+)/, 'get the version string');
 
-my $expected_help = <<"EOF";
-$version
+my $expected_help = qr{opustags version .*\n\nUsage: opustags --help\n};
+cmp_deeply(opustags('--help'), [re($expected_help), '', 0], '--help displays the help message');
+cmp_deeply(opustags('-h'), [re($expected_help), '', 0], '-h displays the help message too');
 
-Usage: opustags --help
-       opustags [OPTIONS] FILE
-       opustags OPTIONS FILE -o FILE
-
-Options:
-  -h, --help                    print this help
-  -o, --output FILE             specify the output file
-  -i, --in-place                overwrite the input file
-  -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
-
-See the man page for extensive documentation.
-EOF
-
-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');
+is_deeply(opustags('--derp'), ['', <<"EOF", 512], 'unrecognized option shows an error');
 error: Unrecognized option '--derp'.
 EOF
 
 is_deeply(opustags('../opustags'), ['', <<"EOF", 256], 'not an Ogg stream');
-error: Input is not a valid Ogg file.
+../opustags: error: Input is not a valid Ogg file.
 EOF
 
 ####################################################################################################
@@ -101,25 +83,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(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
@@ -142,20 +131,24 @@ TITLE=gobble
 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');
+is_deeply(opustags(qw(-i out.opus -a fatal=yes -a FOO -a BAR)), ['', <<'EOF', 512], 'bad tag with --add');
 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
+is_deeply(opustags('out.opus', '-D', '-a', "X=foobar\tquux"), [<<'END_OUT', <<'END_ERR', 0], 'control characters');
+X=foobar	quux
 END_OUT
-warning: Some tags contain newline characters. These are not supported by --set-all.
 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');
+is_deeply(opustags('out.opus', '-D', '-a', "X=foo\n\nbar"), [<<'END_OUT', '', 0], 'newline characters');
+X=foo
+	
+	bar
+END_OUT
+
+is_deeply(opustags(qw(-i out.opus -s fatal=yes -s FOO -s BAR)), ['', <<'EOF', 512], 'bad tag with --set');
 error: Comment does not contain an equal sign: FOO.
 EOF
 is(md5('out.opus'), '66780307a6081523dc9040f3c47b0448', 'the file did not change');
@@ -170,6 +163,7 @@ ARTIST=七面鳥
 
 A=A
 X=Y
+#IGNORE=COMMENTS
 END_IN
 OK=yes again
 ARTIST=七面鳥
@@ -201,6 +195,34 @@ is_deeply(opustags('-', '-o', '-', {in => $data, mode => ':raw'}), [$data, '', 0
 
 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
 
@@ -208,7 +230,7 @@ system('ffmpeg -loglevel error -y -i gobble.opus -c copy -map 0:0 -map 0:0 -shor
 	or BAIL_OUT('could not create a muxed stream');
 
 is_deeply(opustags('muxed.ogg'), ['', <<'END_ERR', 256], 'muxed streams detection');
-error: Muxed streams are not supported yet.
+muxed.ogg: error: Muxed streams are not supported yet.
 END_ERR
 
 unlink('muxed.ogg');
@@ -216,15 +238,16 @@ unlink('muxed.ogg');
 ####################################################################################################
 # Locale
 
-my $locale = 'fr_FR.iso88591';
+my $locale = 'en_US.iso88591';
 my @all_locales = split(' ', `locale -a`);
 
 SKIP: {
-skip "locale $locale is not present", 4 unless (any { $_ eq $locale } @all_locales);
+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
@@ -234,14 +257,16 @@ 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', 0], 'read 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
-TITLE=???
-ARTIST=\xe9\xe0\xe7
 I=\xf9\xce
 END_OUT
-warning: Some tags have been transliterated to your system encoding.
-END_ERR
 
 $ENV{LC_ALL} = '';
 
@@ -251,4 +276,69 @@ 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');
+
+####################################################################################################
+# Multiple-page tags
+
+my $big_tags = "DATA=x\n" x 15000; # > 90K, which is over the max page size of 64KiB.
+is_deeply(opustags(qw(-S gobble.opus -o out.opus), {in => $big_tags}), ['', '', 0], 'write multi-page header');
+is_deeply(opustags('out.opus'), [$big_tags, '', 0], 'read multi-page header');
+is_deeply(opustags(qw(out.opus -i -D -a), 'encoder=Lavc58.18.100 libopus'), ['', '', 0], 'shrink the header');
+is(md5('out.opus'), '111a483596ac32352fbce4d14d16abd2', 'the result is identical to the original file');
+unlink('out.opus');
+
+####################################################################################################
+# Cover arts
+
+is_deeply(opustags(qw(-D --set-cover pixel.png gobble.opus -o out.opus)), ['', '', 0], 'set the cover');
+is_deeply(opustags(qw(--output-cover out.png out.opus)), [<<'END_OUT', '', 0], 'extract the cover');
+METADATA_BLOCK_PICTURE=AAAAAwAAAAlpbWFnZS9wbmcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEWJUE5HDQoaCgAAAA1JSERSAAAAAQAAAAEIAgAAAJB3U94AAAAMSURBVAjXY/j//z8ABf4C/tzMWecAAAAASUVORK5CYII=
+END_OUT
+is(md5('out.png'), md5('pixel.png'), 'the extracted cover is identical to the one set');
+unlink('out.opus');
+unlink('out.png');
+
+is_deeply(opustags(qw(-D --set-cover - gobble.opus), { in => "GIF8 x" }), [<<'END_OUT', '', 0], 'read the cover from stdin');
+METADATA_BLOCK_PICTURE=AAAAAwAAAAlpbWFnZS9naWYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAZHSUY4IHg=
+END_OUT
+
+####################################################################################################
+# Vendor string
+
+is_deeply(opustags(qw(--vendor gobble.opus)), ["Lavf58.12.100\n", '', 0], 'print the vendor string');
+
+is_deeply(opustags(qw(--set-vendor opustags gobble.opus -o out.opus)), ['', '', 0], 'set the vendor string');
+is_deeply(opustags(qw(--vendor out.opus)), ["opustags\n", '', 0], 'the vendor string was updated');
+unlink('out.opus');
+
+####################################################################################################
+# Multi-line tags
+
+is_deeply(opustags(qw(--set-all gobble.opus -o out.opus), { in => "MULTILINE=one\n\ttwo\nSIMPLE=three\n" }), ['', '', 0], 'parses continuation lines');
+is_deeply(opustags(qw(out.opus -z)), ["MULTILINE=one\ntwo\0SIMPLE=three\0", '', 0], 'delimits output with NUL on -z');
+unlink('out.opus');
+
+is_deeply(opustags(qw(--set-all gobble.opus -o out.opus -z), { in => "MULTILINE=one\ntwo\0SIMPLE=three\0" }), ['', '', 0], 'delimits input with NUL on -z');
+is_deeply(opustags(qw(out.opus)), [<<'END', '', 0], 'indents continuation lines');
+MULTILINE=one
+	two
+SIMPLE=three
+END
+unlink('out.opus');

t/system.cc

@@ -10,10 +10,14 @@ void check_partial_files()
 	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");
+		try {
+			bad_tmp.open("/dev/null");
+			throw failure("opening a device as a partial file should fail");
+		} catch (const ot::status& rc) {
+			is(rc, ot::st::standard_error, "opening a device as a partial file fails");
+		}
+
+		bad_tmp.open(result);
 		name = bad_tmp.name();
 		if (name.size() != strlen(result) + 12 ||
 		    name.compare(0, strlen(result), result) != 0)
@@ -22,37 +26,53 @@ void check_partial_files()
 	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");
+	good_tmp.open(result);
 	name = good_tmp.name();
-	is(good_tmp.commit(), ot::st::ok, "commit the result file");
+	good_tmp.commit();
 	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_slurp()
+{
+	static const ot::byte_string_view pixel = ""_bsv
+		"\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d"
+		"\x49\x48\x44\x52\x00\x00\x00\x01\x00\x00\x00\x01"
+		"\x08\x02\x00\x00\x00\x90\x77\x53\xde\x00\x00\x00"
+		"\x0c\x49\x44\x41\x54\x08\xd7\x63\xf8\xff\xff\x3f"
+		"\x00\x05\xfe\x02\xfe\xdc\xcc\x59\xe7\x00\x00\x00"
+		"\x00\x49\x45\x4e\x44\xae\x42\x60\x82";
+	opaque_is(ot::slurp_binary_file("pixel.png"), pixel, "loads a whole 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//TRANSLIT");
-	std::string out;
+	setlocale(LC_ALL, "");
+	is(ot::decode_utf8(ot::encode_utf8("Éphémère")), "Éphémère", "decode_utf8 reverts encode_utf8");
+	opaque_is(ot::encode_utf8(ot::decode_utf8(u8"Éphémère")), u8"Éphémère",
+	          "encode_utf8 reverts decode_utf8");
 
-	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");
+	try {
+		ot::decode_utf8((char8_t*) "\xFF\xFF");
+		throw failure("conversion from bad UTF-8 did not fail");
+	} catch (const ot::status&) {}
+}
 
-	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(2);
+	plan(4);
 	run(check_partial_files, "test partial files");
+	run(check_slurp, "file slurping");
 	run(check_converter, "test encoding converter");
+	run(check_shell_esape, "test shell escaping");
 	return 0;
 }

t/tap.h

@@ -30,6 +30,8 @@ static void run(F test, const char *name)
 		ok = true;
 	} catch (failure& e) {
 		std::cerr << "# fail: " << e.what() << "\n";
+	} catch (const ot::status &rc) {
+		std::cerr << "# unexpected error: " << rc.message << "\n";
 	}
 	std::cout << (ok ? "ok" : "not ok") << " - " << name << "\n";
 }
@@ -49,6 +51,13 @@ void is(const T& got, const U& expected, const char* name)
 	}
 }
 
+template <typename T, typename U>
+void opaque_is(const T& got, const U& expected, const char* name)
+{
+	if (got != expected)
+		throw failure(name);
+}
+
 template <>
 void is(const ot::status& got, const ot::st& expected, const char* name)
 {