[FFmpeg-devel] [PATCH] Add picture attachment support to OGG Muxer

[Michael Niedermayer]

On Sat, Jun 06, 2020 at 04:37:54PM -0500, Lewis Fox wrote:
> Signed-off-by: Lewis Fox <LRFLEW at aol.com>
> ---
> This is an implementation for #4448, adding support for embedding attached cover art to OGG files. I had a need for the feature for a setup I was using, and had written a patch for it about half a year ago. I had forgotten to submit it back then, though. I remembered the patch recently, and spent some time rebasing against the latest master so I could finally put the patch on the mailing list.
> 
> Using the code for attachments on the FLAC container, I got an idea of what would be required to get it to work. I saw that, due to the attachment data being passed to the muxer as a stream, the actual stream data has to be buffered until the attachments are received and written. I figured I would have to implement this buffering for OGG to make attachments work. However, I saw that the OGG muxer already had OGGPageList, which did the same type of buffering (I think to make interlacing streams work). I ended up expanding the use of this class to make inserting attachment data work.
> 
> From what I've seen looking through the code for libavformat, OGG is rather unique in that attachments are stored on a per-stream basis. Everything else I've seen either stores attachments at a container scope or only supports one stream per container. As a result, FFmpeg was never designed around associating attachments to streams. This raised an interesting question of how to determine which stream to place the attachments in (when there are multiple streams). My solution was to add an optional metadata tag for attachments named "ogg_attach". When an attachment stream has this metadata tag, then the OGG muxer will place the attachment in the stream at that index. If the tag isn't present, or is set to an invalid value, it will default to the first stream in the output. To make the process symmetric, I added this metadata tag when demuxing an OGG container with attachments.
> 
> OGG FLAC turned out to be rather tricky to figure out. FLAC supports adding the attachments in the vorbis comments, but also supports directly adding the attachments to the stream (as done with the FLAC muxer). The existing demuxer only looked for attachements in OGG FLAC streams in the vorbis comments, so I investigated adding it there first. The challenge is that FLAC's vorbis comments is the only OGG-supported codec that adds a length value to the vorbis comments packet. All the other codecs just use the length indicated by the OGG structure. Since the size of the attachments isn't known until the muxer receives the attachment data, writing attachments to the vorbis comments would require adding extra complexity to update the length value. Additionally, the length written in the stream is only 24 bits. Since the image data is base64 encoded in vorbis comments, this means that having more than about 12.5 MB of attachments will overflow this value. This is a reasonably reachable size using things like lossless images (PNG), large image resolutions, and multiple images (front cover, back cover, etc.). I opted, for FLAC specifically, to add the attachments as their own metadata blocks, the same way as the FLAC muxer. Since the OGG demuxer didn't look for these metadata blocks, I also updated the demuxer to read them.
> 
> I tested all of the codecs supported by FFmpeg's OGG muxer: Vorbis, FLAC, Speex, Opus, Theora, and VP8 (the latter two with audio channels, with the attachment on either). I also tested having multiple images attached to a single file. Using VLC, I was able to see the cover art for Vorbis, Speex, and Opus, and was able to confirm that all the files except FLAC still played without problems. OGG FLAC appears to have some bugs in VLC, as even without this patch VLC skipped the first about 2 seconds of my 4 second test file. Attaching cover art also caused VLC to not parse the other metadata, such as title, album, and artist. However, due to VLC's buggy handling of OGG FLAC, it's hard to say that it is a problem with this patch. I confirmed that the audio did play normally in other players (including Firefox and Chrome), and FFmpeg can read the metadata without problems (even pre-patch). I tried other audio players, but OGG FLAC is poorly supported, and most that I tried had problems unrelated to the patch. The ones I found that worked well enough used libavcodec/libavformat, so it didn't make for a good test. At this point, I'm happy with how far I was able to take testing. I also looked at the resulting files in a hex editor, and I couldn't find any problems with any of the files I made with this patch.
> 
>  libavformat/flac_picture.c   | 112 +++++++++-
>  libavformat/flac_picture.h   |   6 +-
>  libavformat/flacdec.c        |   4 +-
>  libavformat/flacenc.c        | 100 +--------
>  libavformat/matroskadec.c    |   2 +-
>  libavformat/matroskaenc.c    |   2 +-
>  libavformat/oggdec.h         |   3 +-
>  libavformat/oggenc.c         | 402 +++++++++++++++++++++++++++++------
>  libavformat/oggparseflac.c   |   4 +
>  libavformat/oggparsevorbis.c |   8 +-
>  libavformat/vorbiscomment.c  |  32 +--
>  libavformat/vorbiscomment.h  |   6 +-
>  12 files changed, 503 insertions(+), 178 deletions(-)

seems to break fate

--- ./tests/ref/fate/limited_input_seek-copyts	2020-05-28 14:46:23.971547831 +0200
+++ tests/data/fate/limited_input_seek-copyts	2020-06-07 22:10:38.303050886 +0200
@@ -1 +1 @@
-ffe8a674bdf38e4f650f91963debc654
+aae5603508b268cbb456b633b84a48af
Test limited_input_seek-copyts failed. Look at tests/data/fate/limited_input_seek-copyts.err for details.
tests/Makefile:255: recipe for target 'fate-limited_input_seek-copyts' failed
make: *** [fate-limited_input_seek-copyts] Error 1


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200607/ea020aa7/attachment.sig>