Keep the album art when converting audio to Opus, Ogg Vorbis and MKA (1.5.2)

CHANGELOG.md

@@ -6,6 +6,14 @@ version it publishes under "What's Changed" in the GitHub release notes, above t
 Add a `## <version>` section before tagging a release. Write what someone using Filee notices: what's new, where to
 find it, what it does and doesn't do.
 
+## 1.5.2
+
+### Fixes
+
+- Converting audio to Opus or Ogg Vorbis now keeps the album art. FFmpeg can't put a picture into those files, so
+  Filee writes it the way they store pictures (a METADATA_BLOCK_PICTURE tag); title, artist and the other tags stay
+  too. Matroska audio (MKA) gets the album art as a cover attachment.
+
 ## 1.5.1
 
 ### Fixes

Directory.Build.props

@@ -14,7 +14,7 @@
          parameters of a member is fine (CS1573). -->
     <NoWarn>$(NoWarn);CS1591;CS1573</NoWarn>
 
-    <Version>1.5.1</Version>
+    <Version>1.5.2</Version>
     <Product>Filee</Product>
     <Authors>Filee contributors</Authors>
     <Copyright>Copyright (c) Filee contributors</Copyright>

src/Filee.Engines/Media/CoverArt.cs

@@ -0,0 +1,121 @@
+// Album art for audio formats FFmpeg cannot write a picture stream into. Ogg (Vorbis, Opus) keeps the picture as a
+// METADATA_BLOCK_PICTURE comment: a FLAC picture block in base64, which FFmpeg writes like any other tag when it comes
+// from an ffmetadata file. Matroska audio keeps it as a cover.jpg / cover.png attachment.
+
+using System.Buffers.Binary;
+using System.Text;
+using ImageMagick;
+
+namespace Filee.Engines.Media;
+
+/// <summary>Extra inputs that carry the source's cover into the output (see <see cref="CoverArt.PrepareAsync"/>).</summary>
+/// <param name="MetadataFile">ffmpeg metadata file with the source's tags and the picture comment (Ogg).</param>
+/// <param name="AttachmentFile">The picture to attach (Matroska).</param>
+/// <param name="AttachmentMime">MIME type of <paramref name="AttachmentFile"/>.</param>
+internal sealed record CoverExtras(string? MetadataFile, string? AttachmentFile, string? AttachmentMime);
+
+internal static class CoverArt
+{
+    /// <summary>Targets whose cover is a METADATA_BLOCK_PICTURE comment.</summary>
+    public static bool AsComment(string target) => target is "ogg" or "opus";
+
+    /// <summary>Targets whose cover is an attached file.</summary>
+    public static bool AsAttachment(string target) => target is "mka";
+
+    /// <summary>
+    /// Extracts the cover (stream <paramref name="stream"/>) of <paramref name="input"/> and prepares what
+    /// <paramref name="target"/> needs to carry it. Null when the target doesn't need it or anything fails: the file is
+    /// then converted without its cover rather than not at all.
+    /// </summary>
+    public static async Task<CoverExtras?> PrepareAsync(string ffmpeg, string input, int stream, string target, string workDirectory,
+        CancellationToken cancellationToken)
+    {
+        if (!AsComment(target) && !AsAttachment(target))
+            return null;
+        try
+        {
+            var folder = Path.Combine(workDirectory, "cover-" + Guid.NewGuid().ToString("N")[..8]);
+            Directory.CreateDirectory(folder);
+            var raw = Path.Combine(folder, "cover.bin");
+            if (!await RunAsync(ffmpeg, ["-i", input, "-map", $"0:{stream}", "-c", "copy", "-frames:v", "1", "-f", "image2", raw], cancellationToken))
+                return null;
+            var image = await File.ReadAllBytesAsync(raw, cancellationToken);
+            var mime = MimeOf(image);
+            if (mime is null)
+                return null;
+            var picture = Path.Combine(folder, mime == "image/png" ? "cover.png" : "cover.jpg");
+            File.Move(raw, picture);
+
+            if (AsAttachment(target))
+                return new CoverExtras(null, picture, mime);
+
+            // The source's own tags (title, artist, …) go into the same file: the output takes its tags from it.
+            var metadata = Path.Combine(folder, "metadata.txt");
+            if (!await RunAsync(ffmpeg, ["-i", input, "-f", "ffmetadata", metadata], cancellationToken))
+                return null;
+            var info = new MagickImageInfo(image);
+            var line = "METADATA_BLOCK_PICTURE=" + Escape(PictureBlock(image, mime, (int)info.Width, (int)info.Height));
+            var lines = (await File.ReadAllLinesAsync(metadata, cancellationToken)).ToList();
+            lines.Insert(Math.Min(1, lines.Count), line); // global tags come right after ";FFMETADATA1"
+            await File.WriteAllTextAsync(metadata, string.Join('\n', lines) + "\n", new UTF8Encoding(false), cancellationToken);
+            return new CoverExtras(metadata, null, null);
+        }
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or MagickException or InvalidOperationException)
+        {
+            return null;
+        }
+    }
+
+    /// <summary>
+    /// A FLAC METADATA_BLOCK_PICTURE (front cover) in base64, as Ogg comments carry it: big-endian picture type,
+    /// MIME type, description, width, height, colour depth, palette size and the image itself.
+    /// </summary>
+    internal static string PictureBlock(byte[] image, string mime, int width, int height)
+    {
+        var mimeBytes = Encoding.ASCII.GetBytes(mime);
+        var block = new byte[4 * 8 + mimeBytes.Length + image.Length];
+        var at = 0;
+        void Int(int value)
+        {
+            BinaryPrimitives.WriteInt32BigEndian(block.AsSpan(at), value);
+            at += 4;
+        }
+        Int(3); // front cover
+        Int(mimeBytes.Length);
+        mimeBytes.CopyTo(block, at);
+        at += mimeBytes.Length;
+        Int(0); // no description
+        Int(width);
+        Int(height);
+        Int(24); // colour depth
+        Int(0); // not indexed
+        Int(image.Length);
+        image.CopyTo(block, at);
+        return Convert.ToBase64String(block);
+    }
+
+    /// <summary>ffmetadata escaping: '=', ';', '#', '\' and line breaks get a backslash.</summary>
+    internal static string Escape(string value)
+    {
+        var text = new StringBuilder(value.Length + 16);
+        foreach (var c in value)
+        {
+            if (c is '=' or ';' or '#' or '\\' or '\n')
+                text.Append('\\');
+            text.Append(c);
+        }
+        return text.ToString();
+    }
+
+    /// <summary>JPEG or PNG by their first bytes; null for anything else.</summary>
+    internal static string? MimeOf(byte[] image) =>
+        image.Length > 4 && image[0] == 0xFF && image[1] == 0xD8 ? "image/jpeg"
+        : image.Length > 8 && image[0] == 0x89 && image[1] == (byte)'P' && image[2] == (byte)'N' && image[3] == (byte)'G' ? "image/png"
+        : null;
+
+    private static async Task<bool> RunAsync(string ffmpeg, IReadOnlyList<string> arguments, CancellationToken cancellationToken)
+    {
+        var result = await FfmpegRunner.RunAsync(ffmpeg, [.. MediaEncoding.Common, .. arguments], null, cancellationToken);
+        return result.ExitCode == 0;
+    }
+}

src/Filee.Engines/Media/FfmpegConverter.cs

@@ -77,8 +77,13 @@ public async Task<IReadOnlyList<string>> ConvertAsync(ConversionStep step, IProg
         if (output is null)
             return [];
 
+        // Album art for targets FFmpeg can't put a picture stream into (Ogg, Matroska): prepared on the side.
+        var cover = info.CoverStreamIndex is { } coverStream
+            ? await CoverArt.PrepareAsync(ffmpeg, input, coverStream, step.To, step.WorkDirectory, cancellationToken)
+            : null;
+
         // Throws for a missing stream before anything is written (an existing file of that name stays untouched).
-        var passes = MediaEncoding.Plan(input, output, step.To, step.Preset.Media, info, step.WorkDirectory);
+        var passes = MediaEncoding.Plan(input, output, step.To, step.Preset.Media, info, step.WorkDirectory, cover);
         try
         {
             var done = 0.0;

src/Filee.Engines/Media/MediaEncoding.cs

@@ -97,15 +97,17 @@ private static readonly (int Num, int Den)[] BroadcastRates =
     private static readonly string[] SourceOnlyTags = ["major_brand", "minor_version", "compatible_brands"];
 
     /// <summary>Global arguments of every run: no banner, no keyboard input, only errors on stderr, overwrite.</summary>
-    private static readonly string[] Common = ["-hide_banner", "-nostdin", "-nostats", "-v", "error", "-y"];
+    internal static readonly string[] Common = ["-hide_banner", "-nostdin", "-nostats", "-v", "error", "-y"];
 
     /// <summary>
     /// The ffmpeg runs that convert <paramref name="input"/> into <paramref name="output"/> in format
     /// <paramref name="target"/>.
     /// </summary>
     /// <param name="workDirectory">Scratch folder (the GIF palette goes there).</param>
+    /// <param name="cover">The source's cover for targets FFmpeg can't put a picture stream into (<see cref="CoverArt"/>).</param>
     /// <exception cref="InvalidOperationException">The file lacks the stream the target needs (e.g. no audio track).</exception>
-    public static IReadOnlyList<FfmpegPass> Plan(string input, string output, string target, MediaOptions options, MediaInfo info, string workDirectory)
+    public static IReadOnlyList<FfmpegPass> Plan(string input, string output, string target, MediaOptions options, MediaInfo info, string workDirectory,
+        CoverExtras? cover = null)
     {
         if (target == "gif")
             return AnimatedGif(input, output, options, info, workDirectory);
@@ -114,7 +116,7 @@ public static IReadOnlyList<FfmpegPass> Plan(string input, string output, string
         if (VideoTargets.TryGetValue(target, out var video))
             return [new FfmpegPass(Video(input, output, target, video, options, info), 1)];
         if (AudioTargets.TryGetValue(target, out var audio))
-            return [new FfmpegPass(Audio(input, output, target, audio, options, info), 1)];
+            return [new FfmpegPass(Audio(input, output, target, audio, options, info, cover), 1)];
         throw new NotSupportedException($"FFmpeg cannot write '{target}'.");
     }
 
@@ -260,11 +262,11 @@ internal static string BroadcastRate(double fps)
         _ => [],
     };
 
-    private static IEnumerable<string> MetadataArguments()
+    private static IEnumerable<string> MetadataArguments(int from = 0)
     {
         // Keep title, artist, dates, … (chapters are kept by default); an empty value removes a tag.
         yield return "-map_metadata";
-        yield return "0";
+        yield return from.ToString(CultureInfo.InvariantCulture);
         foreach (var tag in SourceOnlyTags)
         {
             yield return "-metadata";
@@ -274,14 +276,21 @@ private static IEnumerable<string> MetadataArguments()
 
     // ───────────────────────── Audio ─────────────────────────
 
-    private static List<string> Audio(string input, string output, string target, MediaTarget spec, MediaOptions options, MediaInfo info)
+    private static List<string> Audio(string input, string output, string target, MediaTarget spec, MediaOptions options, MediaInfo info,
+        CoverExtras? extras)
     {
         var audio = info.Audio ?? throw new InvalidOperationException("The file has no audio track.");
-        List<string> args = [.. Common, "-i", input, "-map", "0:a:0"];
+        List<string> args = [.. Common, "-i", input];
+        // Ogg: the tags, and the picture as a comment, come from a metadata file (input 1) instead of the source.
+        if (extras?.MetadataFile is { } metadata)
+            args.AddRange(["-i", metadata]);
+        args.AddRange(["-map", "0:a:0"]);
         if (info.CoverStreamIndex is { } cover && CoverTargets.Contains(target))
             args.AddRange(["-map", $"0:{cover}", "-c:v", "copy", "-disposition:v:0", "attached_pic"]);
+        if (extras is { AttachmentFile: { } picture, AttachmentMime: { } mime })
+            args.AddRange(["-attach", picture, "-metadata:s:t", "mimetype=" + mime, "-metadata:s:t", "filename=" + Path.GetFileName(picture)]);
         args.AddRange(AudioCodecArguments(target, spec.AudioCodec, options, audio, inVideo: false));
-        args.AddRange(MetadataArguments());
+        args.AddRange(MetadataArguments(extras?.MetadataFile is null ? 0 : 1));
         args.AddRange(ContainerArguments(target));
         args.AddRange(["-f", spec.Muxer, "-progress", "pipe:1", output]);
         return args;

tests/Filee.Engines.Tests/CoverArtTests.cs

@@ -0,0 +1,39 @@
+// The METADATA_BLOCK_PICTURE comment Ogg files carry their album art in (no FFmpeg needed).
+
+using System.Buffers.Binary;
+using System.Text;
+using Filee.Engines.Media;
+
+namespace Filee.Engines.Tests;
+
+public class CoverArtTests
+{
+    [Fact]
+    public void Picture_block_is_a_flac_front_cover_in_base64()
+    {
+        byte[] jpeg = [0xFF, 0xD8, 0xFF, 0xE0, 1, 2, 3];
+
+        var block = Convert.FromBase64String(CoverArt.PictureBlock(jpeg, "image/jpeg", 300, 200));
+
+        int At(int offset) => BinaryPrimitives.ReadInt32BigEndian(block.AsSpan(offset));
+        Assert.Equal(3, At(0)); // front cover
+        Assert.Equal(10, At(4));
+        Assert.Equal("image/jpeg", Encoding.ASCII.GetString(block, 8, 10));
+        Assert.Equal(0, At(18)); // no description
+        Assert.Equal((300, 200, 24, 0), (At(22), At(26), At(30), At(34)));
+        Assert.Equal(jpeg.Length, At(38));
+        Assert.Equal(jpeg, block[42..]);
+    }
+
+    [Fact]
+    public void Metadata_values_escape_ffmetadata_specials() =>
+        Assert.Equal(@"a\=b\;c\#d\\e", CoverArt.Escape(@"a=b;c#d\e"));
+
+    [Fact]
+    public void Only_jpeg_and_png_covers_are_used()
+    {
+        Assert.Equal("image/jpeg", CoverArt.MimeOf([0xFF, 0xD8, 0xFF, 0xDB, 0]));
+        Assert.Equal("image/png", CoverArt.MimeOf([0x89, (byte)'P', (byte)'N', (byte)'G', 0x0D, 0x0A, 0x1A, 0x0A, 0]));
+        Assert.Null(CoverArt.MimeOf([(byte)'B', (byte)'M', 0, 0, 0]));
+    }
+}

tests/Filee.Engines.Tests/MediaTests.cs

@@ -522,14 +522,16 @@ public async Task Cover_art_and_tags_are_kept()
         await RunAsync(Tools.Value.Ffmpeg, ["-hide_banner", "-nostdin", "-v", "error", "-y", "-f", "lavfi", "-i", "sine=duration=2", "-i", cover,
             "-map", "0:a", "-map", "1:v", "-c:v", "copy", "-disposition:v:0", "attached_pic", "-metadata", "title=노래 제목", song]);
 
-        foreach (var target in new[] { "flac", "m4a" })
+        // FLAC and M4A take a picture stream; Opus and Ogg a METADATA_BLOCK_PICTURE comment; MKA an attachment.
+        foreach (var target in new[] { "flac", "m4a", "opus", "ogg", "mka" })
         {
             var output = await ConvertAsync(song, target);
             var probed = await ProbeAsync(output);
-            Assert.Contains(probed.Streams, s => s.Cover && s.Codec == "png");
-            var tags = await ProcessRunner.RunAsync(Tools.Value.Ffprobe, ["-v", "error", "-show_entries", "format_tags=title,major_brand", "-of", "json", output],
+            Assert.True(probed.Streams.Any(s => s.Cover && s.Codec == "png"), $"{target}: no cover");
+            var tags = await ProcessRunner.RunAsync(Tools.Value.Ffprobe,
+                ["-v", "error", "-show_entries", "format_tags=title,major_brand:stream_tags=title", "-of", "json", output],
                 TimeSpan.FromMinutes(1), CancellationToken.None);
-            Assert.Contains("노래 제목", tags.StandardOutput);
+            Assert.True(tags.StandardOutput.Contains("노래 제목"), $"{target}: title lost");
         }
     }