diff mbox

[FFmpeg-devel] avformat/segafilm - fix keyframe detection and set packet, flags

Message ID e163facc-a46f-3eca-42fb-b4dd7a2d5318@gmail.com
State Superseded
Headers show

Commit Message

Gyan March 21, 2018, 1:37 p.m. UTC
Fix for ticket 7091.

Regards,
Gyan
From 24a68eca54a5051b3386664f07d21358aa0db02d Mon Sep 17 00:00:00 2001
From: Gyan Doshi <gyandoshi@gmail.com>
Date: Wed, 21 Mar 2018 18:59:33 +0530
Subject: [PATCH] avformat/segafilm - fix keyframe detection and set packet
 flags

Streams from a Segafilm cpk file can't be streamcopied because
keyframe flag isn't correctly set in stream index and
said flag is never conveyed to the packet's flags

Fixes #7091
---
 libavformat/segafilm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Carl Eugen Hoyos March 21, 2018, 8:32 p.m. UTC | #1
2018-03-21 14:37 GMT+01:00, Gyan Doshi <gyandoshi@gmail.com>:
> Fix for ticket 7091.

> -            film->sample_table[i].keyframe = (scratch[8] & 0x80) ? 0 : 1;
> +            film->sample_table[i].keyframe = (scratch[8] & 0x80) ? 1 : 0;

Can you comment on why this 15-year old code was wrong?

Could be keyframe = !!(scratch & 0x80)

Carl Eugen
Carl Eugen Hoyos March 22, 2018, 1:09 a.m. UTC | #2
2018-03-21 21:32 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2018-03-21 14:37 GMT+01:00, Gyan Doshi <gyandoshi@gmail.com>:
>> Fix for ticket 7091.
>
>> -            film->sample_table[i].keyframe = (scratch[8] & 0x80) ? 0 :
>> 1;
>> +            film->sample_table[i].keyframe = (scratch[8] & 0x80) ? 1 :
>> 0;
>
> Can you comment on why this 15-year old code was wrong?
>
> Could be keyframe = !!(scratch & 0x80)

Or actually:
film->sample_table[i].keyframe = scratch[8] & 0x80 ? AVINDEX_KEYFRAME : 0;

Carl Eugen
misty@brew.sh March 22, 2018, 2:39 a.m. UTC | #3
On Wed, Mar 21, 2018 at 1:32 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2018-03-21 14:37 GMT+01:00, Gyan Doshi <gyandoshi@gmail.com>:
> Can you comment on why this 15-year old code was wrong?

I think it was de facto unused before now. Since it wasn't passed
through to AVPacket it wasn't used by many things, and the Cinepak
decoder determines keyframe status by looking at the Cinepak packet
header and ignores what the container says.
diff mbox

Patch

diff --git a/libavformat/segafilm.c b/libavformat/segafilm.c
index 1fdef50cc7..11768823fc 100644
--- a/libavformat/segafilm.c
+++ b/libavformat/segafilm.c
@@ -239,7 +239,7 @@  static int film_read_header(AVFormatContext *s)
         } else {
             film->sample_table[i].stream = film->video_stream_index;
             film->sample_table[i].pts = AV_RB32(&scratch[8]) & 0x7FFFFFFF;
-            film->sample_table[i].keyframe = (scratch[8] & 0x80) ? 0 : 1;
+            film->sample_table[i].keyframe = (scratch[8] & 0x80) ? 1 : 0;
             video_frame_counter++;
             if (film->video_type)
                 av_add_index_entry(s->streams[film->video_stream_index],
@@ -286,6 +286,7 @@  static int film_read_packet(AVFormatContext *s,
 
     pkt->stream_index = sample->stream;
     pkt->pts = sample->pts;
+    pkt->flags |= sample->keyframe;
 
     film->current_sample++;