| Message ID | 20190429224559.81159-1-fumoboy007@me.com |
|---|---|
| State | New |
| Headers | show |
Bump. > On Apr 29, 2019, at 3:45 PM, fumoboy007 <fumoboy007@me.com> wrote: > > One situation where a subtitle packet can be marked for discard is when demuxing an MOV file that has an edit list. > --- > libavcodec/decode.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index 6c31166ec2..204bd50fa3 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -1149,8 +1149,14 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, > } > } > > - if (*got_sub_ptr) > - avctx->frame_number++; > + if (*got_sub_ptr) { > + if (avpkt->flags & AV_PKT_FLAG_DISCARD) { > + *got_sub_ptr = 0; > + avsubtitle_free(sub); > + } else { > + avctx->frame_number++; > + } > + } > } > > return ret; > -- > 2.21.0 >
On 29/04/2019 23:45, fumoboy007 wrote: > One situation where a subtitle packet can be marked for discard is when demuxing an MOV file that has an edit list. > --- > libavcodec/decode.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) Will this work properly if a given subtitle overlaps an edit boundary? - Derek
Good question. The subtitle would be discarded if it overlaps an edit boundary. The root cause is the MOV demuxer currently marks boundary packets for discard. However, due to subtitle frames not being discarded (fixed by this patch), the root cause is hidden. A potential fix for the root cause would be to handle boundary packets by changing their timestamps to fit the edit boundary. (That would be a separate patch though.) > On May 22, 2019, at 10:30 AM, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote: > >> On 29/04/2019 23:45, fumoboy007 wrote: >> One situation where a subtitle packet can be marked for discard is when demuxing an MOV file that has an edit list. >> --- >> libavcodec/decode.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) > > Will this work properly if a given subtitle overlaps an edit boundary? > > - Derek > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 22/05/2019 18:55, Darren Mo wrote: > Good question. The subtitle would be discarded if it overlaps an edit boundary. > > The root cause is the MOV demuxer currently marks boundary packets for discard. However, due to subtitle frames not being discarded (fixed by this patch), the root cause is hidden. A potential fix for the root cause would be to handle boundary packets by changing their timestamps to fit the edit boundary. (That would be a separate patch though.) As a start, this patch seems reasonable then... - Derek
To clarify, do you mean we should merge this now or wait for the second patch, which fixes the root cause? > On May 23, 2019, at 10:12 AM, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote: > >> On 22/05/2019 18:55, Darren Mo wrote: >> Good question. The subtitle would be discarded if it overlaps an edit boundary. >> >> The root cause is the MOV demuxer currently marks boundary packets for discard. However, due to subtitle frames not being discarded (fixed by this patch), the root cause is hidden. A potential fix for the root cause would be to handle boundary packets by changing their timestamps to fit the edit boundary. (That would be a separate patch though.) > > As a start, this patch seems reasonable then... > > - Derek > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 23/05/2019 23:58, Darren Mo wrote:
> To clarify, do you mean we should merge this now or wait for the second patch, which fixes the root cause?
I have no strong opinion on it. Unsure which is better for a user experience,
given they'll both be broken in some way.
I'd say merge if you feel it is most correct; nobody has raised any objections.
- Derek
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 6c31166ec2..204bd50fa3 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1149,8 +1149,14 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, } } - if (*got_sub_ptr) - avctx->frame_number++; + if (*got_sub_ptr) { + if (avpkt->flags & AV_PKT_FLAG_DISCARD) { + *got_sub_ptr = 0; + avsubtitle_free(sub); + } else { + avctx->frame_number++; + } + } } return ret;