Ogg/Theora: Duplicate frames dropped when copying Theora streams

[Bernat]

Hi. I'm working on Theora decoding support for Godot. Almost everything is working fine except for some kind of video streams.

Theora encodes duplicate frames (frames identical to the previous one) as empty packets. Those empty packets seem to be discarded by FFmpeg when doing an operation that involves copying a Theora stream. It doesn't happen when encoding.

Example:

ffmpeg -i input.ogv -c:v copy -a:n copy output.ogv

Although I'm not familiar with the code, I've searched the code for the cause and I think I found a possible regression in this commit: ​https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/18f24527eb4b520585e55f922cdbc234aa9f0f18

The following line in the Ogg encoder is dropping empty packets: ​https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavformat/oggenc.c#l693

Additionally, doing copies of copies of copies, increases gradually the size of the file and eventually throws this error:

[ogg @ 0x55c70ba998c0] Broken file, non-keyframe not correctly marked.

I use this command to create a test file for this issue:

ffmpeg -f lavfi -i color=white:320x240:d=1.5 -f lavfi -i color=red:320x240:d=0.1 -f lavfi -i color=cyan:320x240:d=1.5 -f lavfi -i color=green:320x240:d=0.1 -f lavfi -i color=yellow:320x240:d=3 -filter_complex "[0:v] [1:v] [2:v] [3:v] [4:v] concat=n=5:v=1 [v]" -map "[v]" -framerate 50 -codec:v libtheora -qscale:v 5 -g:v 30 test.ogv

This command works correctly with FFmpeg 5.1.6 but with the latest master it suffers from the exact same issue this ticket describes.

I hope this can be easily fixed. Thanks in advance. Cheers.

[James]

Does this fix the issue?

diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c
index 224519a4da..57bf5b3509 100644
--- a/libavformat/oggenc.c
+++ b/libavformat/oggenc.c
@@ -241,7 +241,8 @@ static int ogg_buffer_data(AVFormatContext *s, AVStream *st,
 
         len = FFMIN(size, segments*255);
         page->segments[page->segments_count++] = len - (segments-1)*255;
-        memcpy(page->data+page->size, p, len);
+        if (len)
+            memcpy(page->data+page->size, p, len);
         p += len;
         size -= len;
         i += segments;
@@ -690,7 +691,7 @@ static int ogg_write_packet(AVFormatContext *s, AVPacket *pkt)
     int i;
 
     if (pkt)
-        return pkt->size ? ogg_write_packet_internal(s, pkt) : 0;
+        return pkt->size || !pkt->side_data_elems ? ogg_write_packet_internal(s, pkt) : 0;
 
     for (i = 0; i < s->nb_streams; i++) {
         OGGStreamContext *oggstream = s->streams[i]->priv_data;

[Bernat]

No, it's still dropping duplicate frames.

It seems only packets with size > 0 reach that line. Maybe it's already filtered out when this stage is reached?

[Bernat]

Wait, there's something I overlooked.

I did my encoding tests using FFmpeg 5.1.6 and it worked well, it only failed when copying the video stream. I tried the latest master to see if it failed the same when copying streams but didn't try encoding.

Now, I've tried encoding with the latest master and encoding fails in the same way that it fails when copying streams. But when I apply this patch encoding works well, it only fails when copying streams.

So this patch works, it fixes a regression in master that wasn't in 5.1.6. It just doesn't fix the issue when copying streams.

[Bernat]

Let me put this in a table to ease reading.

V: duplicate frames are correctly encoded.
X: duplicate frames are dropped.

FFmpeg version	encoding	copying
5.1.6	V	X
6.1.2	X	X
master	X	X
master with your patch	V	X

[Bernat]

I'd like to help more but this code base is too dense and unfamiliar to me. I've traced the code and the empty packets seem to be dropped (when copying the stream) in this function: ​https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavformat/demux.c#l1162

[Balling]

Nice ​https://patchwork.ffmpeg.org/project/ffmpeg/patch/20250214031435.1557-1-jamrial@gmail.com/

[Bernat]

Tested applying both latest patches from the mailing list to master and this issue seems completely fixed.

Great job! Thank you!

[James]

Fixed in 84d85e7ad4ace228265af0c8c5caccc0730042fd and 6e26f57f672b05e7b8b052007a83aef99dc81ccb