On Sat, Jun 06, 2020 at 04:37:54PM -0500, Lewis Fox wrote: > Signed-off-by: Lewis Fox <lrf...@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
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
