[FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line
=== PROBLEM === I was trying to record h264 + aac streams from an RTSP server to mp4 file. using this command line: ffmpeg -v verbose -y -i "rtsp:///my_resources" -codec copy -bsf:a aac_adtstoasc test.mp4 FFmpeg then fail to record audio and output this logs: [rtsp @ 0xcda1f0] The profile-level-id field size is invalid (40) [rtsp @ 0xcda1f0] Error parsing AU headers ... [rtsp @ 0xcda1f0] Could not find codec parameters for stream 1 (Audio: aac, 48000 Hz, 1 channels): unspecified sample format In SDP provided by my RTSP server I had this fmtp line: a=fmtp:98 streamType=5; profile-level-id=40; mode=AAC-hbr; config=1188; sizeLength=13; indexLength=3; indexDeltaLength=3; In FFmpeg code, I found a check introduced by commit 24130234cd9dd733116d17b724ea4c8e12ce097a. It disallows values greater than 32 for fmtp line parameters. However, In RFC-6416 (RTP Payload Format for MPEG-4 Audio/Visual Streams) give examples of "profile-level-id" values for AAC, up to 55. Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any limit of size on interger parameters given in fmtp line. === FIX === Instead of prohibit values over 32, I propose to check the possible integer overflow. The use of strtoll allow to check the string validity and the possible overflow. Value is then checked against INT32_MIN and INT32_MAX. Using INT32_MIN/MAX ensure to have the same behavior on 32 or 64 bits platforms. This patch fix my problem and I now can record my RTSP AAC stream to mp4. It has passed the full fate tests suite sucessfully. Signed-off-by: Olivier Maignial --- Changes V6 --> V7: - Use long long int and strtoll. LLONG_MAX is always greather than INT32_MIN while LONG_MAX can be equal to INT32_MAX. It allows to accept full INT32 range. - Avoid to use errno libavformat/rtpdec_mpeg4.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c index 4f70599..f1cbedf 100644 --- a/libavformat/rtpdec_mpeg4.c +++ b/libavformat/rtpdec_mpeg4.c @@ -289,15 +289,18 @@ static int parse_fmtp(AVFormatContext *s, for (i = 0; attr_names[i].str; ++i) { if (!av_strcasecmp(attr, attr_names[i].str)) { if (attr_names[i].type == ATTR_NAME_TYPE_INT) { -int val = atoi(value); -if (val > 32) { +char *end_ptr = NULL; +long long int val = strtoll(value, &end_ptr, 10); +if (end_ptr == value || end_ptr[0] != '\0' || +val < INT32_MIN || val > INT32_MAX) { av_log(s, AV_LOG_ERROR, - "The %s field size is invalid (%d)\n", - attr, val); + "The %s field value is not a valid number, or overflows int32: %s\n", + attr, value); return AVERROR_INVALIDDATA; } + *(int *)((char *)data+ -attr_names[i].offset) = val; +attr_names[i].offset) = (int) val; } else if (attr_names[i].type == ATTR_NAME_TYPE_STR) { char *val = av_strdup(value); if (!val) -- 2.7.4 ___ 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".
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter to disable/enable auto scale
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Linjie Fu > Sent: Friday, July 12, 2019 9:19 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Fu, Linjie > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter to > disable/enable auto scale > > Currently, ffmpeg inserts scale filter in the filter graph to force the whole > decoded stream to scale into the same size with the first frame. It's not > quite > make sense in resolution changing cases if user wants the rawvideo without > any scale. > > Option -reinit_filter 0 could be used to realize similar function, but it > fails > when ifilter has hw_frame_ctx. > > Add auto_scale flag set by -reinit_filter to indicate whether auto inserting > the scale filter in the filter graph. > > Signed-off-by: Linjie Fu > --- > Request for comments. > As we have discussed in the rawdump filter patch, here is a simpler solution > based on -reinit_filter, and reuse this option.(maybe it's not easy to be > accepted to add a separate option) > > fftools/ffmpeg.c| 2 +- > fftools/ffmpeg.h| 1 + > fftools/ffmpeg_filter.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index > 01f04103cf..5305b87bd4 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2133,6 +2133,7 @@ static int ifilter_send_frame(InputFilter *ifilter, > AVFrame *frame) > > /* determine if the parameters for this input changed */ > need_reinit = ifilter->format != frame->format; > +fg->auto_scale = ifilter->ist->reinit_filters; > > switch (ifilter->ist->st->codecpar->codec_type) { > case AVMEDIA_TYPE_AUDIO: > @@ -2145,7 +2146,6 @@ static int ifilter_send_frame(InputFilter *ifilter, > AVFrame *frame) > ifilter->height != frame->height; > break; > } > - > if (!ifilter->ist->reinit_filters && fg->graph) > need_reinit = 0; > > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index > 7b6f802082..0c289b439f 100644 > --- a/fftools/ffmpeg.h > +++ b/fftools/ffmpeg.h > @@ -285,6 +285,7 @@ typedef struct FilterGraph { > > AVFilterGraph *graph; > int reconfiguration; > +int auto_scale; > > InputFilter **inputs; > int nb_inputs; > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index > 72838de1e2..856ba48de7 100644 > --- a/fftools/ffmpeg_filter.c > +++ b/fftools/ffmpeg_filter.c > @@ -469,7 +469,7 @@ static int configure_output_video_filter(FilterGraph > *fg, OutputFilter *ofilter, > if (ret < 0) > return ret; > > -if (ofilter->width || ofilter->height) { > +if ((ofilter->width || ofilter->height) && fg->auto_scale) { > char args[255]; > AVFilterContext *filter; > AVDictionaryEntry *e = NULL; > -- > 2.17.1 Is there any big difference with https://patchwork.ffmpeg.org/patch/8173/, especially for the purpose of disabling scaling for HWACCEL? ___ 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".
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter to disable/enable auto scale
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Fu, Linjie > Sent: Friday, July 12, 2019 1:36 PM > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter > to disable/enable auto scale > > > -Original Message- > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > > Of myp...@gmail.com > > Sent: Friday, July 12, 2019 13:28 > > To: FFmpeg development discussions and patches > de...@ffmpeg.org> > > Cc: Fu, Linjie > > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use > > -reinit_filter to disable/enable auto scale > > > > On Fri, Jul 12, 2019 at 1:19 PM Linjie Fu wrote: > > > > > > Currently, ffmpeg inserts scale filter in the filter graph to force > > > the whole decoded stream to scale into the same size with the first > > > frame. It's not quite make sense in resolution changing cases if > > > user wants the rawvideo without any scale. > > > > > > Option -reinit_filter 0 could be used to realize similar function, > > > but it fails when ifilter has hw_frame_ctx. > > > > > > Add auto_scale flag set by -reinit_filter to indicate whether auto > > > inserting the scale filter in the filter graph. > > > > > > Signed-off-by: Linjie Fu > > > --- > > > Request for comments. > > > As we have discussed in the rawdump filter patch, here is a simpler > > > solution based on -reinit_filter, and reuse this option.(maybe it's > > > not easy to be accepted to add a separate option) > > > > > > fftools/ffmpeg.c| 2 +- > > > fftools/ffmpeg.h| 1 + > > > fftools/ffmpeg_filter.c | 2 +- > > > 3 files changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index > > > 01f04103cf..5305b87bd4 100644 > > > --- a/fftools/ffmpeg.c > > > +++ b/fftools/ffmpeg.c > > > @@ -2133,6 +2133,7 @@ static int ifilter_send_frame(InputFilter > > > *ifilter, > > AVFrame *frame) > > > > > > /* determine if the parameters for this input changed */ > > > need_reinit = ifilter->format != frame->format; > > > +fg->auto_scale = ifilter->ist->reinit_filters; > > > > > > switch (ifilter->ist->st->codecpar->codec_type) { > > > case AVMEDIA_TYPE_AUDIO: > > > @@ -2145,7 +2146,6 @@ static int ifilter_send_frame(InputFilter > > > *ifilter, > > AVFrame *frame) > > > ifilter->height != frame->height; > > > break; > > > } > > > - > > Unrelated change > > Yep, this should be recalled. > > > > > > Add an option to disable/enable "auto insert" is Ok for me, but I > > think if you reuse the -reinit_filter option, you need to update doc > > part at the same time. > > I prefer to add a separate option, too. > Depending on the comments, I'll either update doc for reinit_filter, or add a > new option and related doc. Since there is an existed option named "autorotate", add an new option "autoscale" looks can keep tune. ___ 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".
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter to disable/enable auto scale
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of myp...@gmail.com > Sent: Friday, July 12, 2019 13:28 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Cc: Fu, Linjie > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter > to disable/enable auto scale > > On Fri, Jul 12, 2019 at 1:19 PM Linjie Fu wrote: > > > > Currently, ffmpeg inserts scale filter in the filter graph to force > > the whole decoded stream to scale into the same size with the first > > frame. It's not quite make sense in resolution changing cases if user > > wants the rawvideo without any scale. > > > > Option -reinit_filter 0 could be used to realize similar function, but > > it fails when ifilter has hw_frame_ctx. > > > > Add auto_scale flag set by -reinit_filter to indicate whether auto > > inserting the scale filter in the filter graph. > > > > Signed-off-by: Linjie Fu > > --- > > Request for comments. > > As we have discussed in the rawdump filter patch, here is a simpler > > solution based on -reinit_filter, and reuse this option.(maybe it's not > > easy to be accepted to add a separate option) > > > > fftools/ffmpeg.c| 2 +- > > fftools/ffmpeg.h| 1 + > > fftools/ffmpeg_filter.c | 2 +- > > 3 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index 01f04103cf..5305b87bd4 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -2133,6 +2133,7 @@ static int ifilter_send_frame(InputFilter *ifilter, > AVFrame *frame) > > > > /* determine if the parameters for this input changed */ > > need_reinit = ifilter->format != frame->format; > > +fg->auto_scale = ifilter->ist->reinit_filters; > > > > switch (ifilter->ist->st->codecpar->codec_type) { > > case AVMEDIA_TYPE_AUDIO: > > @@ -2145,7 +2146,6 @@ static int ifilter_send_frame(InputFilter *ifilter, > AVFrame *frame) > > ifilter->height != frame->height; > > break; > > } > > - > Unrelated change Yep, this should be recalled. > > > Add an option to disable/enable "auto insert" is Ok for me, but I > think if you reuse the -reinit_filter option, you need to update doc > part at the same time. I prefer to add a separate option, too. Depending on the comments, I'll either update doc for reinit_filter, or add a new option and related doc. Thanks. ___ 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".
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter to disable/enable auto scale
On Fri, Jul 12, 2019 at 1:19 PM Linjie Fu wrote: > > Currently, ffmpeg inserts scale filter in the filter graph to force > the whole decoded stream to scale into the same size with the first > frame. It's not quite make sense in resolution changing cases if user > wants the rawvideo without any scale. > > Option -reinit_filter 0 could be used to realize similar function, but > it fails when ifilter has hw_frame_ctx. > > Add auto_scale flag set by -reinit_filter to indicate whether auto > inserting the scale filter in the filter graph. > > Signed-off-by: Linjie Fu > --- > Request for comments. > As we have discussed in the rawdump filter patch, here is a simpler > solution based on -reinit_filter, and reuse this option.(maybe it's not > easy to be accepted to add a separate option) > > fftools/ffmpeg.c| 2 +- > fftools/ffmpeg.h| 1 + > fftools/ffmpeg_filter.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 01f04103cf..5305b87bd4 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2133,6 +2133,7 @@ static int ifilter_send_frame(InputFilter *ifilter, > AVFrame *frame) > > /* determine if the parameters for this input changed */ > need_reinit = ifilter->format != frame->format; > +fg->auto_scale = ifilter->ist->reinit_filters; > > switch (ifilter->ist->st->codecpar->codec_type) { > case AVMEDIA_TYPE_AUDIO: > @@ -2145,7 +2146,6 @@ static int ifilter_send_frame(InputFilter *ifilter, > AVFrame *frame) > ifilter->height != frame->height; > break; > } > - Unrelated change > if (!ifilter->ist->reinit_filters && fg->graph) > need_reinit = 0; > > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h > index 7b6f802082..0c289b439f 100644 > --- a/fftools/ffmpeg.h > +++ b/fftools/ffmpeg.h > @@ -285,6 +285,7 @@ typedef struct FilterGraph { > > AVFilterGraph *graph; > int reconfiguration; > +int auto_scale; > > InputFilter **inputs; > int nb_inputs; > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c > index 72838de1e2..856ba48de7 100644 > --- a/fftools/ffmpeg_filter.c > +++ b/fftools/ffmpeg_filter.c > @@ -469,7 +469,7 @@ static int configure_output_video_filter(FilterGraph *fg, > OutputFilter *ofilter, > if (ret < 0) > return ret; > > -if (ofilter->width || ofilter->height) { > +if ((ofilter->width || ofilter->height) && fg->auto_scale) { > char args[255]; > AVFilterContext *filter; > AVDictionaryEntry *e = NULL; > -- > 2.17.1 > Add an option to disable/enable "auto insert" is Ok for me, but I think if you reuse the -reinit_filter option, you need to update doc part at the same time. ___ 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".
[FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: use -reinit_filter to disable/enable auto scale
Currently, ffmpeg inserts scale filter in the filter graph to force the whole decoded stream to scale into the same size with the first frame. It's not quite make sense in resolution changing cases if user wants the rawvideo without any scale. Option -reinit_filter 0 could be used to realize similar function, but it fails when ifilter has hw_frame_ctx. Add auto_scale flag set by -reinit_filter to indicate whether auto inserting the scale filter in the filter graph. Signed-off-by: Linjie Fu --- Request for comments. As we have discussed in the rawdump filter patch, here is a simpler solution based on -reinit_filter, and reuse this option.(maybe it's not easy to be accepted to add a separate option) fftools/ffmpeg.c| 2 +- fftools/ffmpeg.h| 1 + fftools/ffmpeg_filter.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 01f04103cf..5305b87bd4 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -2133,6 +2133,7 @@ static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame) /* determine if the parameters for this input changed */ need_reinit = ifilter->format != frame->format; +fg->auto_scale = ifilter->ist->reinit_filters; switch (ifilter->ist->st->codecpar->codec_type) { case AVMEDIA_TYPE_AUDIO: @@ -2145,7 +2146,6 @@ static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame) ifilter->height != frame->height; break; } - if (!ifilter->ist->reinit_filters && fg->graph) need_reinit = 0; diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index 7b6f802082..0c289b439f 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -285,6 +285,7 @@ typedef struct FilterGraph { AVFilterGraph *graph; int reconfiguration; +int auto_scale; InputFilter **inputs; int nb_inputs; diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 72838de1e2..856ba48de7 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -469,7 +469,7 @@ static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter, if (ret < 0) return ret; -if (ofilter->width || ofilter->height) { +if ((ofilter->width || ofilter->height) && fg->auto_scale) { char args[255]; AVFilterContext *filter; AVDictionaryEntry *e = NULL; -- 2.17.1 ___ 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".
Re: [FFmpeg-devel] [PATCH 4/4] avcodec/dirac_parser: Fix overflow in dts
On 7/11/2019 6:49 PM, Michael Niedermayer wrote: > Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type > 'int' > Fixes: > 15568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5634719611355136 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/dirac_parser.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/dirac_parser.c b/libavcodec/dirac_parser.c > index 1ade44a438..8722ef17b7 100644 > --- a/libavcodec/dirac_parser.c > +++ b/libavcodec/dirac_parser.c > @@ -214,7 +214,7 @@ static int dirac_combine_frame(AVCodecParserContext *s, > AVCodecContext *avctx, >pc->index - 13 - pu1.prev_pu_offset; > int pts = AV_RB32(cur_pu + 13); > if (s->last_pts == 0 && s->last_dts == 0) > -s->dts = pts - 1; > +s->dts = pts - 1LL; Unless that AV_RB32() value can be negative in valid bitstreams, just make pts int64_t instead. That's the type for both pts and dts in AVCodecParserContext. > else > s->dts = s->last_dts + 1; > s->pts = pts; > ___ 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".
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/mpc8: Fixes invalid shift in mpc8_decode_frame()
On 7/11/2019 6:49 PM, Michael Niedermayer wrote: > Fixes: left shift of negative value -456 > Fixes: > 15561/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPC8_fuzzer-5758130404720640 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/mpc8.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/mpc8.c b/libavcodec/mpc8.c > index 3be2f79a5a..75943064da 100644 > --- a/libavcodec/mpc8.c > +++ b/libavcodec/mpc8.c > @@ -364,7 +364,7 @@ static int mpc8_decode_frame(AVCodecContext * avctx, void > *data, > for(j = 0; j < SAMPLES_PER_BAND; j += SAMPLES_PER_BAND / 2){ > cnt = get_vlc2(gb, q1_vlc.table, MPC8_Q1_BITS, 2); > t = mpc8_get_mask(gb, 18, cnt); > -for(k = 0; k < SAMPLES_PER_BAND / 2; k++, t <<= 1) > +for(k = 0; k < SAMPLES_PER_BAND / 2; k++, t += > (unsigned)t) How about something like this instead (Untested) for(k = 0; k < SAMPLES_PER_BAND / 2; k++) c->Q[ch][off + j + k] = t & (1 << (SAMPLES_PER_BAND / 2 - k - 1)) ? (get_bits1(gb) << 1) - 1 : 0; > c->Q[ch][off + j + k] = (t & 0x2) ? > (get_bits1(gb) << 1) - 1 : 0; > } > break; > ___ 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".
Re: [FFmpeg-devel] [PATCH 2/3] lavf/webm_chunk: Fix NULL dereference
Andreas Rheinhardt: > Andreas Rheinhardt: >> Andreas Rheinhardt: >>> Andreas Rheinhardt: The earlier version of the webm_chunk muxer had several bugs: 1. If the first packet of an audio stream didn't have a PTS of zero, then no chunk will be started before a packet is delivered to the underlying Matroska/WebM muxer, i.e. the AVFormatContext used to write these packets had a NULL as AVIOContext for output. This is behind the crash in ticket #5752. 2. If an error happens during writing a packet, the underlyimg Matroska/WebM muxer context is freed. This leads to a use-after-free coupled with a double-free in webm_chunk_write_trailer (which supposes that the underlying AVFormatContext is still valid). 3. Even when no error occurs at all, webm_chunk_write_trailer is still buggy: After the underlying Matroska/WebM muxer has written its trailer, ending the chunk implicitly flushes it again which is illegal at this point. These bugs have been fixed. Fixes #5752. Signed-off-by: Andreas Rheinhardt --- libavformat/webm_chunk.c | 44 +--- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c index 2c99753b5b..391fee721a 100644 --- a/libavformat/webm_chunk.c +++ b/libavformat/webm_chunk.c @@ -168,7 +168,7 @@ static int chunk_start(AVFormatContext *s) return 0; } -static int chunk_end(AVFormatContext *s) +static int chunk_end(AVFormatContext *s, int flush) { WebMChunkContext *wc = s->priv_data; AVFormatContext *oc = wc->avf; @@ -179,11 +179,14 @@ static int chunk_end(AVFormatContext *s) char filename[MAX_FILENAME_SIZE]; AVDictionary *options = NULL; -if (wc->chunk_start_index == wc->chunk_index) +if (!oc->pb) return 0; -// Flush the cluster in WebM muxer. -oc->oformat->write_packet(oc, NULL); + +if (flush) +// Flush the cluster in WebM muxer. +oc->oformat->write_packet(oc, NULL); buffer_size = avio_close_dyn_buf(oc->pb, &buffer); +oc->pb = NULL; ret = get_chunk_filename(s, 0, filename); if (ret < 0) goto fail; @@ -194,7 +197,6 @@ static int chunk_end(AVFormatContext *s) goto fail; avio_write(pb, buffer, buffer_size); ff_format_io_close(s, &pb); -oc->pb = NULL; fail: av_dict_free(&options); av_free(buffer); @@ -216,27 +218,19 @@ static int webm_chunk_write_packet(AVFormatContext *s, AVPacket *pkt) } // For video, a new chunk is started only on key frames. For audio, a new -// chunk is started based on chunk_duration. -if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && +// chunk is started based on chunk_duration. Also, a new chunk is started +// unconditionally if there is no currently open chunk. +if (!oc->pb || (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && (pkt->flags & AV_PKT_FLAG_KEY)) || (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && - (pkt->pts == 0 || wc->duration_written >= wc->chunk_duration))) { + wc->duration_written >= wc->chunk_duration)) { wc->duration_written = 0; -if ((ret = chunk_end(s)) < 0 || (ret = chunk_start(s)) < 0) { -goto fail; +if ((ret = chunk_end(s, 1)) < 0 || (ret = chunk_start(s)) < 0) { +return ret; } } ret = oc->oformat->write_packet(oc, pkt); -if (ret < 0) -goto fail; - -fail: -if (ret < 0) { -oc->streams = NULL; -oc->nb_streams = 0; -avformat_free_context(oc); -} return ret; } @@ -245,12 +239,20 @@ static int webm_chunk_write_trailer(AVFormatContext *s) { WebMChunkContext *wc = s->priv_data; AVFormatContext *oc = wc->avf; +int ret; + +if (!oc->pb) { +ret = chunk_start(s); +if (ret < 0) +goto fail; +} oc->oformat->write_trailer(oc); -chunk_end(s); +ret = chunk_end(s, 0); +fail: oc->streams = NULL; oc->nb_streams = 0; avformat_free_context(oc); -return 0; +return ret; } #define OFFSET(x) offsetof(WebMChunkContext, x) >>> Ping for the last two patches of this patchset (i.e. this one and >>> https://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242902.html) >>> >>> - Andreas >>> >> And another ping for these two patches. >> >> - Andreas
[FFmpeg-devel] [PATCH 3/4] tools/target_dec_fuzzer: Remove redundant av_free()
Signed-off-by: Michael Niedermayer --- tools/target_dec_fuzzer.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c index a2ef94139b..07ad59c63e 100644 --- a/tools/target_dec_fuzzer.c +++ b/tools/target_dec_fuzzer.c @@ -281,9 +281,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { av_frame_free(&frame); avcodec_free_context(&ctx); -av_freep(&ctx); avcodec_free_context(&parser_avctx); -av_freep(&parser_avctx); av_parser_close(parser); FDBDesroy(&buffer); return 0; -- 2.22.0 ___ 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".
[FFmpeg-devel] [PATCH 1/4] avcodec/mpc8: Fixes invalid shift in mpc8_decode_frame()
Fixes: left shift of negative value -456 Fixes: 15561/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPC8_fuzzer-5758130404720640 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/mpc8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/mpc8.c b/libavcodec/mpc8.c index 3be2f79a5a..75943064da 100644 --- a/libavcodec/mpc8.c +++ b/libavcodec/mpc8.c @@ -364,7 +364,7 @@ static int mpc8_decode_frame(AVCodecContext * avctx, void *data, for(j = 0; j < SAMPLES_PER_BAND; j += SAMPLES_PER_BAND / 2){ cnt = get_vlc2(gb, q1_vlc.table, MPC8_Q1_BITS, 2); t = mpc8_get_mask(gb, 18, cnt); -for(k = 0; k < SAMPLES_PER_BAND / 2; k++, t <<= 1) +for(k = 0; k < SAMPLES_PER_BAND / 2; k++, t += (unsigned)t) c->Q[ch][off + j + k] = (t & 0x2) ? (get_bits1(gb) << 1) - 1 : 0; } break; -- 2.22.0 ___ 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".
[FFmpeg-devel] [PATCH 4/4] avcodec/dirac_parser: Fix overflow in dts
Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int' Fixes: 15568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5634719611355136 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/dirac_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/dirac_parser.c b/libavcodec/dirac_parser.c index 1ade44a438..8722ef17b7 100644 --- a/libavcodec/dirac_parser.c +++ b/libavcodec/dirac_parser.c @@ -214,7 +214,7 @@ static int dirac_combine_frame(AVCodecParserContext *s, AVCodecContext *avctx, pc->index - 13 - pu1.prev_pu_offset; int pts = AV_RB32(cur_pu + 13); if (s->last_pts == 0 && s->last_dts == 0) -s->dts = pts - 1; +s->dts = pts - 1LL; else s->dts = s->last_dts + 1; s->pts = pts; -- 2.22.0 ___ 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".
[FFmpeg-devel] [PATCH 2/4] avcodec/interplayvideo: Avoid ff_get_buffer() during init
This is unneeded for interplay video Fixes: memleak Fixes: 15562/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_INTERPLAY_VIDEO_fuzzer-5162268645392384 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/interplayvideo.c | 8 1 file changed, 8 deletions(-) diff --git a/libavcodec/interplayvideo.c b/libavcodec/interplayvideo.c index 4313fdf7ac..c26ffbaee7 100644 --- a/libavcodec/interplayvideo.c +++ b/libavcodec/interplayvideo.c @@ -1181,14 +1181,6 @@ static av_cold int ipvideo_decode_init(AVCodecContext *avctx) s->cur_decode_frame->format = avctx->pix_fmt; s->prev_decode_frame->format = avctx->pix_fmt; -ret = ff_get_buffer(avctx, s->cur_decode_frame, 0); -if (ret < 0) -goto error; - -ret = ff_get_buffer(avctx, s->prev_decode_frame, 0); -if (ret < 0) -goto error; - return 0; error: av_frame_free(&s->last_frame); -- 2.22.0 ___ 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".
Re: [FFmpeg-devel] [PATCH] Add support for Display Definition Segment to DVB Subtitle encoder
To answer your questions... 1.) Yes, 2 is the version. To be honest I didn't really know what it meant. I had a working sample stream & used the same values. But after reading the spec I can see it's only used to signal changes in the display definition segment which means it could be anything between 0 and 15 as it's going to stay constant anyways. I can set it to 0 or 1 if you that's what you prefer. 2.) I saw that I could just pass avctx & read the priv_data inside encode_dvb_subtitles function. I was just trying to make the least amount of changes. But I'll change this and push a new patch tomorrow. Regards, Jernej On Thu, Jul 11, 2019 at 10:01 PM Marton Balint wrote: > > > On Thu, 11 Jul 2019, mikroh...@gmail.com wrote: > > > Current version of dvbsub encoder doesn't support HD DVB subtitles. The > high resolution bitmaps are muxed into the stream but without the DDS > (display definition segment) the players asume that the DVB subtitles are > in SD (720x576) resolution which causes them to either render the subtitles > too large and misplaced or don't render them at all. By including the DDS > as defined in section 7.7.1 of ETSI EN 300 743 (V1.3.1) this problem is > fixed. > > > > 7.2.1 Display definition segment > > The display definition for a subtitle service may be defined by the > display definition segment if present in the stream. > > Absence of a DDS implies that the stream is coded in accordance with EN > 300 743 (V1.2.1) [5] and that a display width of 720 pixels and a display > height of 576 lines may be assumed. > > > > > https://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf > > > > Signed-off-by: Jernej Fijacko > > --- > > libavcodec/dvbsub.c | 18 -- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/dvbsub.c b/libavcodec/dvbsub.c > > index 8cce702..c838567 100644 > > --- a/libavcodec/dvbsub.c > > +++ b/libavcodec/dvbsub.c > > @@ -247,7 +247,8 @@ static void dvb_encode_rle8(uint8_t **pq, > > *pq = q; > > } > > > > -static int encode_dvb_subtitles(DVBSubtitleContext *s, > > +static int encode_dvb_subtitles(AVCodecContext *avctx, > > +DVBSubtitleContext *s, > > uint8_t *outbuf, const AVSubtitle *h) > > { > > uint8_t *q, *pseg_len; > > @@ -261,6 +262,19 @@ static int encode_dvb_subtitles(DVBSubtitleContext > *s, > > if (h->num_rects && !h->rects) > > return -1; > > > > +if (avctx->width > 0 && avctx->height > 0) { > > +/* display definition segment */ > > +*q++ = 0x0f; /* sync_byte */ > > +*q++ = 0x14; /* segment_type */ > > +bytestream_put_be16(&q, page_id); > > +pseg_len = q; > > +q += 2; /* segment length */ > > +*q++ = 0x20; /* dds version number & display window flag */ > > I guess 2 is the version number, right? Why 2? > > > +bytestream_put_be16(&q, avctx->width - 1); /* display width */ > > +bytestream_put_be16(&q, avctx->height - 1); /* display height */ > > +bytestream_put_be16(&pseg_len, q - pseg_len - 2); > > +} > > + > > /* page composition segment */ > > > > *q++ = 0x0f; /* sync_byte */ > > @@ -449,7 +463,7 @@ static int dvbsub_encode(AVCodecContext *avctx, > > DVBSubtitleContext *s = avctx->priv_data; > > int ret; > > > > -ret = encode_dvb_subtitles(s, buf, sub); > > +ret = encode_dvb_subtitles(avctx, s, buf, sub); > > You could pass avctx only and assign s the same way as it is assigned > here... > > > return ret; > > Otherwise looks good. > > Thanks, > Marton > ___ > 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". ___ 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".
Re: [FFmpeg-devel] [PATCH] Add support for Display Definition Segment to DVB Subtitle encoder
On Thu, 11 Jul 2019, mikroh...@gmail.com wrote: Current version of dvbsub encoder doesn't support HD DVB subtitles. The high resolution bitmaps are muxed into the stream but without the DDS (display definition segment) the players asume that the DVB subtitles are in SD (720x576) resolution which causes them to either render the subtitles too large and misplaced or don't render them at all. By including the DDS as defined in section 7.7.1 of ETSI EN 300 743 (V1.3.1) this problem is fixed. 7.2.1 Display definition segment The display definition for a subtitle service may be defined by the display definition segment if present in the stream. Absence of a DDS implies that the stream is coded in accordance with EN 300 743 (V1.2.1) [5] and that a display width of 720 pixels and a display height of 576 lines may be assumed. https://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf Signed-off-by: Jernej Fijacko --- libavcodec/dvbsub.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/libavcodec/dvbsub.c b/libavcodec/dvbsub.c index 8cce702..c838567 100644 --- a/libavcodec/dvbsub.c +++ b/libavcodec/dvbsub.c @@ -247,7 +247,8 @@ static void dvb_encode_rle8(uint8_t **pq, *pq = q; } -static int encode_dvb_subtitles(DVBSubtitleContext *s, +static int encode_dvb_subtitles(AVCodecContext *avctx, +DVBSubtitleContext *s, uint8_t *outbuf, const AVSubtitle *h) { uint8_t *q, *pseg_len; @@ -261,6 +262,19 @@ static int encode_dvb_subtitles(DVBSubtitleContext *s, if (h->num_rects && !h->rects) return -1; +if (avctx->width > 0 && avctx->height > 0) { +/* display definition segment */ +*q++ = 0x0f; /* sync_byte */ +*q++ = 0x14; /* segment_type */ +bytestream_put_be16(&q, page_id); +pseg_len = q; +q += 2; /* segment length */ +*q++ = 0x20; /* dds version number & display window flag */ I guess 2 is the version number, right? Why 2? +bytestream_put_be16(&q, avctx->width - 1); /* display width */ +bytestream_put_be16(&q, avctx->height - 1); /* display height */ +bytestream_put_be16(&pseg_len, q - pseg_len - 2); +} + /* page composition segment */ *q++ = 0x0f; /* sync_byte */ @@ -449,7 +463,7 @@ static int dvbsub_encode(AVCodecContext *avctx, DVBSubtitleContext *s = avctx->priv_data; int ret; -ret = encode_dvb_subtitles(s, buf, sub); +ret = encode_dvb_subtitles(avctx, s, buf, sub); You could pass avctx only and assign s the same way as it is assigned here... return ret; Otherwise looks good. Thanks, Marton ___ 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".
Re: [FFmpeg-devel] [PATCHv2] avformat/movenc: use unspecified language by default
On Tue, 9 Jul 2019, Marton Balint wrote: English was used before. Will apply this soon. Regards, Marton Signed-off-by: Marton Balint --- Changelog | 1 + libavformat/movenc.c | 2 +- libavformat/version.h | 2 +- tests/ref/acodec/alac | 2 +- tests/ref/acodec/pcm-s16be | 2 +- tests/ref/acodec/pcm-s24be | 2 +- tests/ref/acodec/pcm-s32be | 2 +- tests/ref/acodec/pcm-s8| 2 +- tests/ref/fate/adtstoasc_ticket3715| 2 +- tests/ref/lavf/mov | 6 +++--- tests/ref/lavf/mov_rtphint | 2 +- tests/ref/vsynth/vsynth1-avui | 2 +- tests/ref/vsynth/vsynth1-dnxhd-1080i | 2 +- tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth1-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth1-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth1-dnxhd-hr-lb-mov | 2 +- tests/ref/vsynth/vsynth1-dnxhd-hr-sq-mov | 2 +- tests/ref/vsynth/vsynth1-mov-bgr24 | 2 +- tests/ref/vsynth/vsynth1-mov-bpp15 | 2 +- tests/ref/vsynth/vsynth1-mov-bpp16 | 2 +- tests/ref/vsynth/vsynth1-prores| 2 +- tests/ref/vsynth/vsynth1-prores_444| 2 +- tests/ref/vsynth/vsynth1-prores_444_int| 2 +- tests/ref/vsynth/vsynth1-prores_int| 2 +- tests/ref/vsynth/vsynth1-prores_ks | 2 +- tests/ref/vsynth/vsynth1-qtrle | 2 +- tests/ref/vsynth/vsynth1-qtrlegray | 2 +- tests/ref/vsynth/vsynth1-svq1 | 2 +- tests/ref/vsynth/vsynth1-vc2-420p | 2 +- tests/ref/vsynth/vsynth1-vc2-420p10| 2 +- tests/ref/vsynth/vsynth1-vc2-420p12| 2 +- tests/ref/vsynth/vsynth1-vc2-422p | 2 +- tests/ref/vsynth/vsynth1-vc2-422p10| 2 +- tests/ref/vsynth/vsynth1-vc2-422p12| 2 +- tests/ref/vsynth/vsynth1-vc2-444p | 2 +- tests/ref/vsynth/vsynth1-vc2-444p10| 2 +- tests/ref/vsynth/vsynth1-vc2-444p12| 2 +- tests/ref/vsynth/vsynth1-vc2-t5_3 | 2 +- tests/ref/vsynth/vsynth1-vc2-thaar | 2 +- tests/ref/vsynth/vsynth2-avui | 2 +- tests/ref/vsynth/vsynth2-dnxhd-1080i | 2 +- tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth2-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth2-dnxhd-hr-lb-mov | 2 +- tests/ref/vsynth/vsynth2-dnxhd-hr-sq-mov | 2 +- tests/ref/vsynth/vsynth2-mov-bgr24 | 2 +- tests/ref/vsynth/vsynth2-mov-bpp15 | 2 +- tests/ref/vsynth/vsynth2-mov-bpp16 | 2 +- tests/ref/vsynth/vsynth2-prores| 2 +- tests/ref/vsynth/vsynth2-prores_444| 2 +- tests/ref/vsynth/vsynth2-prores_444_int| 2 +- tests/ref/vsynth/vsynth2-prores_int| 2 +- tests/ref/vsynth/vsynth2-prores_ks | 2 +- tests/ref/vsynth/vsynth2-qtrle | 2 +- tests/ref/vsynth/vsynth2-qtrlegray | 2 +- tests/ref/vsynth/vsynth2-svq1 | 2 +- tests/ref/vsynth/vsynth2-vc2-420p | 2 +- tests/ref/vsynth/vsynth2-vc2-420p10| 2 +- tests/ref/vsynth/vsynth2-vc2-420p12| 2 +- tests/ref/vsynth/vsynth2-vc2-422p | 2 +- tests/ref/vsynth/vsynth2-vc2-422p10| 2 +- tests/ref/vsynth/vsynth2-vc2-422p12| 2 +- tests/ref/vsynth/vsynth2-vc2-444p | 2 +- tests/ref/vsynth/vsynth2-vc2-444p10| 2 +- tests/ref/vsynth/vsynth2-vc2-444p12| 2 +- tests/ref/vsynth/vsynth2-vc2-t5_3 | 2 +- tests/ref/vsynth/vsynth2-vc2-thaar | 2 +- tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth3-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth3-dnxhd-hr-lb-mov | 2 +- tests/ref/vsynth/vsynth3-dnxhd-hr-sq-mov | 2 +- tests/ref/vsynth/vsynth3-mov-bgr24 | 2 +- tests/ref/vsynth/vsynth3-mov-bpp15 | 2 +- tests/ref/vsynth/vsynth3-mov-bpp16 | 2 +- tests/ref/vsynth/vsynth3-prores| 2 +- tests/ref/vsynth/vsynth3-prores_444| 2 +- tests/ref/vsynth/vsynth3-prores_444_int| 2 +- tests/ref/vsynth/vsynth3-prores_int| 2 +- tests/ref/vsynth/vsynth3-prores_ks | 2 +- tests/ref/vsynth/vsynth3-qtrle | 2 +- tests/ref/vsynth/vsynth3-svq1 | 2 +- tests/ref/vsynth/vsynth_lena-avui | 2 +- tests/ref/vsynth/vsynth_lena-dnxhd-1080i | 2 +- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth_lena-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth_lena-dnxhd-hr-lb-
[FFmpeg-devel] [PATCH] Add support for Display Definition Segment to DVB Subtitle encoder
Current version of dvbsub encoder doesn't support HD DVB subtitles. The high resolution bitmaps are muxed into the stream but without the DDS (display definition segment) the players asume that the DVB subtitles are in SD (720x576) resolution which causes them to either render the subtitles too large and misplaced or don't render them at all. By including the DDS as defined in section 7.7.1 of ETSI EN 300 743 (V1.3.1) this problem is fixed. 7.2.1 Display definition segment The display definition for a subtitle service may be defined by the display definition segment if present in the stream. Absence of a DDS implies that the stream is coded in accordance with EN 300 743 (V1.2.1) [5] and that a display width of 720 pixels and a display height of 576 lines may be assumed. https://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf Signed-off-by: Jernej Fijacko --- libavcodec/dvbsub.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/libavcodec/dvbsub.c b/libavcodec/dvbsub.c index 8cce702..c838567 100644 --- a/libavcodec/dvbsub.c +++ b/libavcodec/dvbsub.c @@ -247,7 +247,8 @@ static void dvb_encode_rle8(uint8_t **pq, *pq = q; } -static int encode_dvb_subtitles(DVBSubtitleContext *s, +static int encode_dvb_subtitles(AVCodecContext *avctx, +DVBSubtitleContext *s, uint8_t *outbuf, const AVSubtitle *h) { uint8_t *q, *pseg_len; @@ -261,6 +262,19 @@ static int encode_dvb_subtitles(DVBSubtitleContext *s, if (h->num_rects && !h->rects) return -1; +if (avctx->width > 0 && avctx->height > 0) { +/* display definition segment */ +*q++ = 0x0f; /* sync_byte */ +*q++ = 0x14; /* segment_type */ +bytestream_put_be16(&q, page_id); +pseg_len = q; +q += 2; /* segment length */ +*q++ = 0x20; /* dds version number & display window flag */ +bytestream_put_be16(&q, avctx->width - 1); /* display width */ +bytestream_put_be16(&q, avctx->height - 1); /* display height */ +bytestream_put_be16(&pseg_len, q - pseg_len - 2); +} + /* page composition segment */ *q++ = 0x0f; /* sync_byte */ @@ -449,7 +463,7 @@ static int dvbsub_encode(AVCodecContext *avctx, DVBSubtitleContext *s = avctx->priv_data; int ret; -ret = encode_dvb_subtitles(s, buf, sub); +ret = encode_dvb_subtitles(avctx, s, buf, sub); return ret; } -- 2.10.2 ___ 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".
[FFmpeg-devel] Support for HD DVB Subtitles
Current version of dvbsub encoder doesn't support HD DVB subtitles. The high resolution bitmaps are muxed into the stream but without the DDS (display definition segment) the players asume that the DVB subtitles are in SD (720x576) resolution which causes them to either render the subtitles too large and misplaced or don't render them at all. By including the DDS as defined in section 7.7.1 of ETSI EN 300 743 (V1.3.1) this problem is fixed. 7.2.1 Display definition segment The display definition for a subtitle service may be defined by the display definition segment if present in the stream. Absence of a DDS implies that the stream is coded in accordance with EN 300 743 (V1.2.1) [5] and that a display width of 720 pixels and a display height of 576 lines may be assumed. https://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf Regards, Jernej 0001-Add-support-for-Display-Definition-Segment-to-DVB-Su.patch Description: Binary data ___ 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".
[FFmpeg-devel] [PATCH] libavcodec/dvbsub: Support for HD DVB Subtitles
Current version of dvbsub encoder doesn't support HD DVB subtitles. The high resolution bitmaps are muxed into the stream but without the DDS (display definition segment) the players asume that the DVB subtitles are in SD (720x576) resolution which causes them to either render the subtitles too large and misplaced or don't render them at all. By including the DDS as defined in section 7.7.1 of ETSI EN 300 743 (V1.3.1) this problem is fixed. 7.2.1 Display definition segment The display definition for a subtitle service may be defined by the display definition segment if present in the stream. Absence of a DDS implies that the stream is coded in accordance with EN 300 743 (V1.2.1) [5] and that a display width of 720 pixels and a display height of 576 lines may be assumed. https://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf Regards, Jernej 0001-Add-support-for-Display-Definition-Segment-to-DVB-Su.patch Description: Binary data ___ 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".
Re: [FFmpeg-devel] [PATCH 1/3] tools/target_dec_fuzzer: fix memleak of extradata
On 7/11/2019 9:29 AM, Michael Niedermayer wrote: > Fixes: memleak > Fixes: > 15535/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKER_fuzzer-5692162424963072 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > tools/target_dec_fuzzer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c > index f456db0e7b..a2ef94139b 100644 > --- a/tools/target_dec_fuzzer.c > +++ b/tools/target_dec_fuzzer.c > @@ -201,7 +201,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t > size) { > > int res = avcodec_open2(ctx, c, NULL); > if (res < 0) { > -av_free(ctx); > +avcodec_free_context(&ctx); Wasn't this fixed by b1febda061? > av_free(parser_avctx); > return 0; // Failure of avcodec_open2() does not imply that a issue > was found > } > ___ 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".
Re: [FFmpeg-devel] [PATCH, v3] lavf/vf_vpp_qsv: add support for QSV transpose filter
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Linjie Fu > Sent: Thursday, July 11, 2019 1:58 AM > To: ffmpeg-devel@ffmpeg.org > Cc: Fu, Linjie > Subject: [FFmpeg-devel] [PATCH, v3] lavf/vf_vpp_qsv: add support for QSV > transpose filter > > Add transpose support for qsv_vpp with rotate and hflip: > - rotate: [0, 3] support clockwise rotation of 0, 90, 180, 270; > - hflip: [0, 1] support horizontal flip; > > Configure with: > {"cclock_hflip","clock","cclock","clock_hflip","reversal","hflip","vflip"} > > Limitation: > If pipeline contains resize, mirroring and other, VPP skips other filters > in > MSDK when IOPattern equals d3d->d3d. So "cclock_hflip, clock_hflip, vflip" > will not work in d3d->d3d condition. > > This pr is fixing this: > https://github.com/Intel-Media-SDK/MediaSDK/pull/1491 > > CMD: > ffmpeg -hwaccel qsv -c:v h264_qsv -i input.h264 > -vf 'format=qsv,vpp_qsv=transpose=clock' -c:v h264_qsv output.h264 > > ffmpeg -init_hw_device qsv=hw -filter_hw_device hw -c:v h264_qsv -i > input.h264 > -vf > 'hwupload=extra_hw_frames=64,format=qsv,vpp_qsv=transpose=cclock_hfli > p' > -f rawvideo -pix_fmt nv12 ./transpose.yuv > > Signed-off-by: Linjie Fu > --- > libavfilter/vf_vpp_qsv.c | 101 > ++- > 1 file changed, 99 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c index > dd05e8baff..01ff42a677 100644 > --- a/libavfilter/vf_vpp_qsv.c > +++ b/libavfilter/vf_vpp_qsv.c > @@ -36,12 +36,15 @@ > #include "libavformat/avformat.h" > > #include "qsvvpp.h" > +#include "transpose.h" > > #define OFFSET(x) offsetof(VPPContext, x) #define FLAGS > (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_FILTERING_PARAM) > > /* number of video enhancement filters */ -#define ENH_FILTERS_COUNT > (5) > +#define ENH_FILTERS_COUNT (7) > +#define QSV_HAVE_ROTATION QSV_VERSION_ATLEAST(1, 17) #define > +QSV_HAVE_MIRRORING QSV_VERSION_ATLEAST(1, 19) > > typedef struct VPPContext{ > const AVClass *class; > @@ -54,6 +57,8 @@ typedef struct VPPContext{ > mfxExtVPPDenoise denoise_conf; > mfxExtVPPDetail detail_conf; > mfxExtVPPProcAmp procamp_conf; > +mfxExtVPPRotation rotation_conf; > +mfxExtVPPMirroring mirroring_conf; > > int out_width; > int out_height; > @@ -70,6 +75,10 @@ typedef struct VPPContext{ > int crop_x; > int crop_y; > > +int transpose; > +int rotate; /* rotate angle : [0, 90, 180, 270] */ > +int hflip; /* flip mode : 0 = off, 1 = HORIZONTAL > flip */ > + > /* param for the procamp */ > intprocamp;/* enable procamp */ > float hue; > @@ -95,6 +104,15 @@ static const AVOption options[] = { > { "contrast","ProcAmp contrast", > OFFSET(contrast),AV_OPT_TYPE_FLOAT,{ .dbl = 1.0 }, 0.0, > 10.0, .flags = FLAGS}, > { "brightness", "ProcAmp brightness", > OFFSET(brightness), AV_OPT_TYPE_FLOAT,{ .dbl = 0.0 }, -100.0, > 100.0, .flags = FLAGS}, > > +{ "transpose", "set transpose direction", OFFSET(transpose), > AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 6, FLAGS, "transpose"}, > +{ "cclock_hflip", "rotate counter-clockwise with horizontal flip", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_CCLOCK_FLIP }, .flags=FLAGS, .unit = "transpose" }, > +{ "clock", "rotate clockwise", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_CLOCK }, .flags=FLAGS, .unit = "transpose" }, > +{ "cclock","rotate counter-clockwise", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_CCLOCK }, .flags=FLAGS, .unit = "transpose" }, > +{ "clock_hflip", "rotate clockwise with horizontal flip", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_CLOCK_FLIP }, .flags=FLAGS, .unit = "transpose" }, > +{ "reversal", "rotate by half-turn", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_REVERSAL}, .flags=FLAGS, .unit = "transpose" }, > +{ "hflip", "flip horizontally", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_HFLIP }, .flags=FLAGS, .unit = "transpose" }, > +{ "vflip", "flip vertically", > 0, AV_OPT_TYPE_CONST, { .i64 = > TRANSPOSE_VFLIP }, .flags=FLAGS, .unit = "transpose" }, > + > { "cw", "set the width crop area expression", OFFSET(cw), > AV_OPT_TYPE_STRING, { .str = "iw" }, CHAR_MIN, CHAR_MAX, FLAGS }, > { "ch", "set the height crop area expression", OFFSET(ch), > AV_OPT_TYPE_STRING, { .str = "ih" }, CHAR_MIN, CHAR_MAX, FLAGS }, > { "cx", "set the x crop area expression", OFFSET(cx), > AV_OPT_TYPE_STRING, { .str = "(in_w-out_w)/2" }, CHAR_MIN, CHAR_MAX, > FLAGS }, > @@ -322,8 +340,87 @@ static int config_output(AVFilterLink *outlink) > param.ext_buf[param.num_ext_buf++] = > (mfxExtBuffer*)&vpp->procamp_conf; > } > > +if (vpp->transpose >= 0) { > +#ifdef QSV_HAVE_ROTATION > +
[FFmpeg-devel] [PATCH 3/3] avcodec/vc1_pred: Fix invalid shift in scaleforsame()
Fixes: left shift of negative value -1 Fixes: 15531/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1IMAGE_fuzzer-5759556258365440 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/vc1_pred.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/vc1_pred.c b/libavcodec/vc1_pred.c index 9e29b44a1f..16565063ea 100644 --- a/libavcodec/vc1_pred.c +++ b/libavcodec/vc1_pred.c @@ -178,7 +178,7 @@ static av_always_inline int scaleforsame(VC1Context *v, int i, int n /* MV */, brfd = FFMIN(v->brfd, 3); scalesame = ff_vc1_b_field_mvpred_scales[0][brfd]; -n = (n * scalesame >> 8) << hpel; +n = (n * scalesame >> 8) * (1 << hpel); return n; } -- 2.22.0 ___ 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".
[FFmpeg-devel] [PATCH 1/3] tools/target_dec_fuzzer: fix memleak of extradata
Fixes: memleak Fixes: 15535/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKER_fuzzer-5692162424963072 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- tools/target_dec_fuzzer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c index f456db0e7b..a2ef94139b 100644 --- a/tools/target_dec_fuzzer.c +++ b/tools/target_dec_fuzzer.c @@ -201,7 +201,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { int res = avcodec_open2(ctx, c, NULL); if (res < 0) { -av_free(ctx); +avcodec_free_context(&ctx); av_free(parser_avctx); return 0; // Failure of avcodec_open2() does not imply that a issue was found } -- 2.22.0 ___ 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".
[FFmpeg-devel] [PATCH 2/3] avcodec/pngdec: Check that previous_picture has same w/h/format
Fixes: out of array access Fixes: 15540/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5684905029140480 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/pngdec.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c index 5209e342db..bf5a5191cc 100644 --- a/libavcodec/pngdec.c +++ b/libavcodec/pngdec.c @@ -1391,6 +1391,9 @@ exit_loop: if (CONFIG_PNG_DECODER && avctx->codec_id != AV_CODEC_ID_APNG) handle_p_frame_png(s, p); else if (CONFIG_APNG_DECODER && + s->previous_picture.f->width == p->width && + s->previous_picture.f->height== p->height && + s->previous_picture.f->format== p->format && avctx->codec_id == AV_CODEC_ID_APNG && (ret = handle_p_frame_apng(avctx, s, p)) < 0) goto fail; -- 2.22.0 ___ 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".
[FFmpeg-devel] [PATCH v4] avfilter/vaapi: add overlay_vaapi filter
--- configure | 1 + libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_overlay_vaapi.c | 424 + 4 files changed, 427 insertions(+) create mode 100644 libavfilter/vf_overlay_vaapi.c diff --git a/configure b/configure index 32fc26356c..f469e6a3b1 100755 --- a/configure +++ b/configure @@ -3478,6 +3478,7 @@ openclsrc_filter_deps="opencl" overlay_opencl_filter_deps="opencl" overlay_qsv_filter_deps="libmfx" overlay_qsv_filter_select="qsvvpp" +overlay_vaapi_filter_deps="vaapi" owdenoise_filter_deps="gpl" pan_filter_deps="swresample" perspective_filter_deps="gpl" diff --git a/libavfilter/Makefile b/libavfilter/Makefile index 07ea8d7edc..ccaad0d6a4 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -311,6 +311,7 @@ OBJS-$(CONFIG_OVERLAY_FILTER)+= vf_overlay.o framesync.o OBJS-$(CONFIG_OVERLAY_OPENCL_FILTER) += vf_overlay_opencl.o opencl.o \ opencl/overlay.o framesync.o OBJS-$(CONFIG_OVERLAY_QSV_FILTER)+= vf_overlay_qsv.o framesync.o +OBJS-$(CONFIG_OVERLAY_VAAPI_FILTER) += vf_overlay_vaapi.o framesync.o vaapi_vpp.o OBJS-$(CONFIG_OWDENOISE_FILTER) += vf_owdenoise.o OBJS-$(CONFIG_PAD_FILTER)+= vf_pad.o OBJS-$(CONFIG_PALETTEGEN_FILTER) += vf_palettegen.o diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c index 9c846b1ddd..27ee1df78b 100644 --- a/libavfilter/allfilters.c +++ b/libavfilter/allfilters.c @@ -295,6 +295,7 @@ extern AVFilter ff_vf_oscilloscope; extern AVFilter ff_vf_overlay; extern AVFilter ff_vf_overlay_opencl; extern AVFilter ff_vf_overlay_qsv; +extern AVFilter ff_vf_overlay_vaapi; extern AVFilter ff_vf_owdenoise; extern AVFilter ff_vf_pad; extern AVFilter ff_vf_palettegen; diff --git a/libavfilter/vf_overlay_vaapi.c b/libavfilter/vf_overlay_vaapi.c new file mode 100644 index 00..9fffa0fcb9 --- /dev/null +++ b/libavfilter/vf_overlay_vaapi.c @@ -0,0 +1,424 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ +#include + +#include "libavutil/avassert.h" +#include "libavutil/mem.h" +#include "libavutil/opt.h" +#include "libavutil/pixdesc.h" + +#include "avfilter.h" +#include "framesync.h" +#include "formats.h" +#include "internal.h" +#include "vaapi_vpp.h" + +typedef struct OverlayVAAPIContext { +VAAPIVPPContext vpp_ctx; // must be the first field +FFFrameSync fs; +int overlay_x; +int overlay_y; +int overlay_w; +int overlay_h; +floatoverlay_alpha; +} OverlayVAAPIContext; + +static int overlay_vaapi_query_formats(AVFilterContext *ctx) +{ +int i; +int ret; + +static const enum AVPixelFormat main_in_fmts[] = { +AV_PIX_FMT_VAAPI, +AV_PIX_FMT_NONE +}; +static const enum AVPixelFormat out_pix_fmts[] = { +AV_PIX_FMT_VAAPI, +AV_PIX_FMT_NONE +}; + +for (i = 0; i < ctx->nb_inputs; i++) { +ret = ff_formats_ref(ff_make_format_list(main_in_fmts), &ctx->inputs[i]->out_formats); +if (ret < 0) +return ret; +} + +ret = ff_formats_ref(ff_make_format_list(out_pix_fmts), &ctx->outputs[0]->in_formats); +if (ret < 0) +return ret; + +return 0; +} + +static int overlay_vaapi_render(AVFilterContext *avctx, +VAProcPipelineParameterBuffer *params, +VAProcPipelineParameterBuffer *subpic_params, +VASurfaceID output_surface) +{ +VABufferID params_id; +VABufferID subpic_params_id; +VAStatus vas; +int err = 0; +VAAPIVPPContext *ctx = avctx->priv; + +vas = vaBeginPicture(ctx->hwctx->display, + ctx->va_context, output_surface); +if (vas != VA_STATUS_SUCCESS) { +av_log(avctx, AV_LOG_ERROR, "Failed to attach new picture: " + "%d (%s).\n", vas, vaErrorStr(vas)); +err = AVERROR(EIO); +goto fail; +} + +vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context, + VAProcPipelineParameterBufferType, +
Re: [FFmpeg-devel] [PATCH, v3] lavc/pthread_frame: update context in child thread in multi-thread mode
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Hendrik Leppkes > Sent: Friday, June 28, 2019 08:56 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v3] lavc/pthread_frame: update > context in child thread in multi-thread mode > > On Thu, Jun 27, 2019 at 1:56 PM Linjie Fu wrote: > > > > Currently in ff_thread_decode_frame, context is updated from child > thread > > to user thread, and user thread releases the context in avcodec_close() > > when decode finishes. > > > > However, when resolution/format changes, ff_get_format is called, and > > hwaccel_uninit() and hwaccel_init will be used to destroy and re-create > > the context. Due to the async between user-thread and child-thread, > > user-thread updates its context from child earlier than the context > > is refreshed in child-thread. And it will lead to: > > 1. memory leak in child-thread. > > 2. double free in user-thread while calling avcodec_close(). > > > > Can be reproduced with a resolution change case, and use -vframes > > to terminate the decode between the dynamic resolution changing frames: > > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v > > verbose -i ./test2360_1672_4980.ivf -pix_fmt p010le -f rawvideo -vsync > > passthrough -vframes 6 -y out.yuv > > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v verbose -i > > ./reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 -f rawvideo > > -vsync passthrough -vframes 45 -y out.yuv > > > > Move update_context_from_thread from ff_thread_decode_frame(user > thread) > > to frame_worker_thread(child thread), update the context in child thread > > right after the context is refreshed to avoid the async issue. > > > > I think the undelying issue that Michael mentioned remains - you are > changing the user context from a worker thread, at which point the > user might be accessing his context simultaneously. You cannot prevent > that with a mutex, since the user does not use your mutex. User context could be accessed everywhere in the user thread, so changing from worker thread will always lead to a race condition. Seems I didn't fully understood this before. Async feature is designed on purpose, it's hard to capture the changes in worker thread in time and get the context updated for user thread correctly. As the async issue exists commonly for resolution changing, it is in need for fixing. > Additionally, the user context should reflect the state of the last > frame that was output to the user, if a worker thread changes it > immediately as it sees the size change, wouldn't it be possible for > frames of the old size to be output after the context was already > updated? That sounds like trouble to me. I didn't quite understand it, but as another patch for vp9 shows, output frame whose size Is different from context can still be correct. Anyway, since it's not easy(or possible) to modify in worker thread, this won't be a problem. Thanks, - linjie ___ 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".
[FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context
VP9 allows resolution changes per frame. Currently in VAAPI, resolution changes leads to va context destroy and reinit. This will cause reference frame surface lost and produce garbage. Though refs surface id could be passed to media driver and found in RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the new created VaContext could only got an empty RefList. As libva allows re-create surface separately without changing the context, this issue could be handled by only recreating hw_frames_ctx. Set hwaccel_priv_data_keeping flag for vp9 to only recreating hw_frame_ctx when dynamic resolution changing happens. Could be verified by: ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv Signed-off-by: Linjie Fu --- libavcodec/decode.c| 10 +- libavcodec/internal.h | 1 + libavcodec/pthread_frame.c | 2 ++ libavcodec/vaapi_decode.c | 40 ++-- libavcodec/vaapi_vp9.c | 4 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 0863b82..7b15fa5 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1254,7 +1254,6 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx, frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data; - if (frames_ctx->initial_pool_size) { // We guarantee 4 base work surfaces. The function above guarantees 1 // (the absolute minimum), so add the missing count. @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx, return AVERROR_PATCHWELCOME; } -if (hwaccel->priv_data_size) { +if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) { avctx->internal->hwaccel_priv_data = av_mallocz(hwaccel->priv_data_size); if (!avctx->internal->hwaccel_priv_data) @@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx, const enum AVPixelFormat *fmt) memcpy(choices, fmt, (n + 1) * sizeof(*choices)); for (;;) { -// Remove the previous hwaccel, if there was one. -hwaccel_uninit(avctx); - +// Remove the previous hwaccel, if there was one, +// and no need for keeping. +if (!avctx->internal->hwaccel_priv_data_keeping) +hwaccel_uninit(avctx); user_choice = avctx->get_format(avctx, choices); if (user_choice == AV_PIX_FMT_NONE) { // Explicitly chose nothing, give up. diff --git a/libavcodec/internal.h b/libavcodec/internal.h index 5096ffa..7adef08 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -188,6 +188,7 @@ typedef struct AVCodecInternal { * hwaccel-specific private data */ void *hwaccel_priv_data; +int hwaccel_priv_data_keeping; /** * checks API usage: after codec draining, flush is required to resume operation diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 36ac0ac..6032818 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -283,6 +283,7 @@ static int update_context_from_thread(AVCodecContext *dst, AVCodecContext *src, dst->sample_fmt = src->sample_fmt; dst->channel_layout = src->channel_layout; dst->internal->hwaccel_priv_data = src->internal->hwaccel_priv_data; +dst->internal->hwaccel_priv_data_keeping = src->internal->hwaccel_priv_data_keeping; if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx || (dst->hw_frames_ctx && dst->hw_frames_ctx->data != src->hw_frames_ctx->data)) { @@ -340,6 +341,7 @@ static int update_context_from_user(AVCodecContext *dst, AVCodecContext *src) dst->frame_number = src->frame_number; dst->reordered_opaque = src->reordered_opaque; dst->thread_safe_callbacks = src->thread_safe_callbacks; +dst->internal->hwaccel_priv_data_keeping = src->internal->hwaccel_priv_data_keeping; if (src->slice_count && src->slice_offset) { if (dst->slice_count < src->slice_count) { diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c index 69512e1..13f0cf0 100644 --- a/libavcodec/vaapi_decode.c +++ b/libavcodec/vaapi_decode.c @@ -613,8 +613,10 @@ int ff_vaapi_decode_init(AVCodecContext *avctx) VAStatus vas; int err; -ctx->va_config = VA_INVALID_ID; -ctx->va_context = VA_INVALID_ID; +if (!ctx->va_config && !ctx->va_context){ +ctx->va_config = VA_INVALID_ID; +ctx->va_context = VA_INVALID_ID; +} #if FF_API_STRUCT_VAAPI_CONTEXT if (avctx->hwaccel_context) { @@ -642,7 +644,6 @@ int ff_vaapi_decode_init(AVCodecContext *avctx) // present, so set it here to avoid the behaviour changing. ctx->hwctx->driver_quirks = AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS; - } #endif @@ -655,7 +656,8 @@ int ff_vaapi_decode_init(AVCodecContext *avctx) "
[FFmpeg-devel] [PATCH 1/2] lavc/decode: recreate hw_frames_ctx instead of return if already exists
If hw_frames_ctx exists when calling ff_decode_get_hw_frames_ctx, it is allowed to be recreated instead of just return. Move hw_frames_ctx check outside ff_decode_get_hw_frames_ctx, and check in relevant code. Signed-off-by: Linjie Fu --- libavcodec/decode.c | 2 +- libavcodec/dxva2.c | 8 +--- libavcodec/vdpau.c | 9 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 6c31166..0863b82 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1230,7 +1230,7 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx, return AVERROR(ENOSYS); if (avctx->hw_frames_ctx) -return 0; +av_buffer_unref(&avctx->hw_frames_ctx); if (!avctx->hw_device_ctx) { av_log(avctx, AV_LOG_ERROR, "A hardware frames or device context is " "required for hardware accelerated decoding.\n"); diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c index 3241611..0404064 100644 --- a/libavcodec/dxva2.c +++ b/libavcodec/dxva2.c @@ -661,9 +661,11 @@ int ff_dxva2_decode_init(AVCodecContext *avctx) // (avctx->pix_fmt is not updated yet at this point) sctx->pix_fmt = avctx->hwaccel->pix_fmt; -ret = ff_decode_get_hw_frames_ctx(avctx, dev_type); -if (ret < 0) -return ret; +if (!avctx->hw_frames_ctx) { +ret = ff_decode_get_hw_frames_ctx(avctx, dev_type); +if (ret < 0) +return ret; +} frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data; sctx->device_ctx = frames_ctx->device_ctx; diff --git a/libavcodec/vdpau.c b/libavcodec/vdpau.c index 167f06d..b7a4e9c 100644 --- a/libavcodec/vdpau.c +++ b/libavcodec/vdpau.c @@ -178,10 +178,11 @@ int ff_vdpau_common_init(AVCodecContext *avctx, VdpDecoderProfile profile, AVHWFramesContext *frames_ctx; AVVDPAUDeviceContext *dev_ctx; -ret = ff_decode_get_hw_frames_ctx(avctx, AV_HWDEVICE_TYPE_VDPAU); -if (ret < 0) -return ret; - +if (!avctx->hw_frames_ctx) { +ret = ff_decode_get_hw_frames_ctx(avctx, AV_HWDEVICE_TYPE_VDPAU); +if (ret < 0) +return ret; +} frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data; dev_ctx = frames_ctx->device_ctx->hwctx; -- 2.7.4 ___ 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".
Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate hw_frames_ctx without destroy va_context
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Fu, Linjie > Sent: Monday, July 8, 2019 16:11 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate > hw_frames_ctx without destroy va_context > > > -Original Message- > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > > Of Yan Wang > > Sent: Monday, July 8, 2019 15:54 > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate > > hw_frames_ctx without destroy va_context > > > > > > On 7/8/2019 2:45 PM, Yan Wang wrote: > > > > > > On 7/7/2019 9:49 PM, Fu, Linjie wrote: > > >>> -Original Message- > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > > Behalf > > >>> Of Mark Thompson > > >>> Sent: Sunday, July 7, 2019 19:51 > > >>> To: ffmpeg-devel@ffmpeg.org > > >>> Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate > > >>> hw_frames_ctx without destroy va_context > > >>> > > >>> On 07/07/2019 17:38, Linjie Fu wrote: > > VP9 allows resolution changes per frame. Currently in VAAPI, > > resolution > > changes leads to va context destroy and reinit. > > >>> Which is correct - it needs to remake the context because the old > > >>> one is for > > >>> the wrong resolution. > > >> It seems that we don't need to remake context, remaking the surface > > >> is enough > > >> for resolution changing. > > >> Currently in libva, surface is able to be recreated separately > > >> without remaking context. > > >> And this way is used in libyami to cope with resolution changing cases. > > >> > > This will cause > > reference frame surface lost and produce garbage. > > >>> This isn't right - the reference frame surfaces aren't lost. See > > >>> VP9Context.s.refs[], which holds references to the frames (including > > >>> their > > >>> hardware frame contexts) until the stream indicates that they can be > > >>> discarded by overwriting their reference frame slots. > > >> I'm not quite sure about this, at least the result shows they are not > > >> used correctly > > >> in current way. > > >> Will look deeper into it. > > > > > > In vaapi_vp9_start_frame() of libavcodec/vaapi_vp9.c, it only passes > > > VASurfaceID into pic_param.reference_frames[i]. > > > > > > But when destroy va_context, the surface/render target based on this > > > VASurfaceID has been destroyed. > > > > Update: the surface isn't destroyed when destroy va_context. But every > > va_context maintains one independent map table: m_ddiDecodeCtx- > >RTtbl. > > > > So the new context cannot find this surface in its map table. > > > > My previous suggested solution should be available still. > > > > Yes, tracing the code and could find that: > > 1. surface is not destroyed until the decode finishes and calls > avcodec_close; > 2. refs[i] is passed to driver, however in DdiMediaBase::GetRenderTargetID, > driver failed to > find the expected surface in the re-created m_ddiDecodeCtx->RTtbl, and > returns invalid. > if(rtTbl->pRT[i] == surface) { > return i; > } > return DDI_CODEC_INVALID_FRAME_INDEX; > > One possible way is to register the refs[] surfaces in the new created context > ->RTtbl and make it > findable. Attempt to register refs[] surface when re-creating context, and observed that refs[] surface has been registered into RTtbl and is able to accessed in driver. https://github.com/fulinjie/ffmpeg/commit/9ed1a309786d2e395753ed327b511e6d9779986f However, the decode still failed to get the correct image. Working together with Wang Yan to investigate more in driver. And Yan found iHD driver will maintain some internal variables and reference link list (CodeclhalDecodeVp9::m_vp9RefList) in Codec HAL layer which will be destroyed when destroy VAAPI context. CodechalDecodeVp9:SetFrameStates() will set resRefPic/dwFrameWidth/dwFrameHeight of CodeclhalDecodeVp9::m_vp9RefList. And CodechalDecodeVp9::~ CodechalDecodeVp9() will destroy it when VAAPI context is destroyed. So the new VAAPI context will only include an empty CodeclhalDecodeVp9::m_vp9RefList. It seems that passing surface ids to driver when recreate context could not fix the dynamic resolution changing issue simply. Refined a new patch to be less hacky. And it fixes more than 1000 dynamic resolution changing cases during my test. It's obvious that currently context in driver is not designed for remap internal variable from previous context. If driver could not cope with this easily, I think we may make it more robust in ffmpeg vaapi vp9. - linjie ___ 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 "unsubscrib