Message ID | DM5PR22MB0681D91B82A83B47C41B6B53FE390@DM5PR22MB0681.namprd22.prod.outlook.com |
---|---|
State | Superseded |
Headers | show |
On Sat, Dec 2, 2017 at 12:55 AM, Colin NG <colin_ng@hotmail.com> wrote: > --- > fftools/ffprobe.c | 5 +++++ > libavcodec/decode.c | 5 ++++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c > index 0e7a771..20b64ef 100644 > --- a/fftools/ffprobe.c > +++ b/fftools/ffprobe.c > @@ -2276,10 +2276,14 @@ static av_always_inline int process_frame(WriterContext *w, > > case AVMEDIA_TYPE_SUBTITLE: > ret = avcodec_decode_subtitle2(dec_ctx, &sub, &got_frame, pkt); > + if (ret == AVERROR(EINVAL) || ret == AVERROR_INVALIDDATA) { > + ret = 0; > + } Ignoring certain error codes doesn't seem correct. If decoding of a subtitle fails, it should be handled somewhere, not just ignored. > @@ -2290,6 +2294,7 @@ static av_always_inline int process_frame(WriterContext *w, > if (got_frame) { > int is_sub = (par->codec_type == AVMEDIA_TYPE_SUBTITLE); > nb_streams_frames[pkt->stream_index]++; > + got_frame = 0; > if (do_show_frames) > if (is_sub) > show_subtitle(w, &sub, ifile->streams[pkt->stream_index].st, fmt_ctx); > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index 3f5b086..d6cc671 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -1024,7 +1024,10 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, > if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE) > sub->pts = av_rescale_q(avpkt->pts, > avctx->pkt_timebase, AV_TIME_BASE_Q); > - ret = avctx->codec->decode(avctx, sub, got_sub_ptr, &pkt_recoded); > + ret = avctx->codec->decode(avctx, sub, &pkt_recoded.size, &pkt_recoded); > + if (ret == avpkt->size) { > + *got_sub_ptr = 1; > + } > av_assert1((ret >= 0) >= !!*got_sub_ptr && > !!*got_sub_ptr >= !!sub->num_rects); > This looks definitely wrong. The decode API has a "*got_frame" ptr there, not any size parameter, and additionally just because it consumed data it does not mean any data was actually output. - Hendrik
On Sat, Dec 2, 2017 at 1:05 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > >> @@ -2290,6 +2294,7 @@ static av_always_inline int process_frame(WriterContext *w, >> if (got_frame) { >> int is_sub = (par->codec_type == AVMEDIA_TYPE_SUBTITLE); >> nb_streams_frames[pkt->stream_index]++; >> + got_frame = 0; Having looked at the logic some more, this part seems to be the key (although implemented too broadly). We don't want to repeat the loop for subtitles, so you set got_frame = 0 here - but you should only do it for subtitles, not for everyhting. - Hendrik
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index 0e7a771..20b64ef 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -2276,10 +2276,14 @@ static av_always_inline int process_frame(WriterContext *w, case AVMEDIA_TYPE_SUBTITLE: ret = avcodec_decode_subtitle2(dec_ctx, &sub, &got_frame, pkt); + if (ret == AVERROR(EINVAL) || ret == AVERROR_INVALIDDATA) { + ret = 0; + } *packet_new = 0; break; default: *packet_new = 0; + break; } } else { *packet_new = 0; @@ -2290,6 +2294,7 @@ static av_always_inline int process_frame(WriterContext *w, if (got_frame) { int is_sub = (par->codec_type == AVMEDIA_TYPE_SUBTITLE); nb_streams_frames[pkt->stream_index]++; + got_frame = 0; if (do_show_frames) if (is_sub) show_subtitle(w, &sub, ifile->streams[pkt->stream_index].st, fmt_ctx); diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 3f5b086..d6cc671 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1024,7 +1024,10 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE) sub->pts = av_rescale_q(avpkt->pts, avctx->pkt_timebase, AV_TIME_BASE_Q); - ret = avctx->codec->decode(avctx, sub, got_sub_ptr, &pkt_recoded); + ret = avctx->codec->decode(avctx, sub, &pkt_recoded.size, &pkt_recoded); + if (ret == avpkt->size) { + *got_sub_ptr = 1; + } av_assert1((ret >= 0) >= !!*got_sub_ptr && !!*got_sub_ptr >= !!sub->num_rects);