Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h264_syntax_template: fix off by 1 error with slice_group_change_cycle
On Mon, Mar 23, 2020 at 12:30:31AM -0300, James Almer wrote: > On 3/22/2020 8:00 PM, Michael Niedermayer wrote: > > Fixes: assertion failure > > Fixes: > > 20390/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5683400772157440 > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/cbs_h264_syntax_template.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/cbs_h264_syntax_template.c > > b/libavcodec/cbs_h264_syntax_template.c > > index 878d348b94..b7e796e9a8 100644 > > --- a/libavcodec/cbs_h264_syntax_template.c > > +++ b/libavcodec/cbs_h264_syntax_template.c > > @@ -1365,7 +1365,7 @@ static int FUNC(slice_header)(CodedBitstreamContext > > *ctx, RWContext *rw, > > pic_size = (sps->pic_width_in_mbs_minus1 + 1) * > > (sps->pic_height_in_map_units_minus1 + 1); > > max = (pic_size + pps->slice_group_change_rate_minus1) / > > - (pps->slice_group_change_rate_minus1 + 1); > > + (pps->slice_group_change_rate_minus1 + 1) + 1; > > The spec says > > "The value of slice_group_change_cycle is represented in the bitstream > by the following number of bits Ceil( Log2( PicSizeInMapUnits ÷ > SliceGroupChangeRate + 1 ) ) > > The value of slice_group_change_cycle shall be in the range of 0 to > Ceil( PicSizeInMapUnits ÷ SliceGroupChangeRate ), inclusive." > > Are you trying to change the amount of bits read, the max allowed value, > or both? the bits, ill send a better patch thanks -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope signature.asc Description: PGP signature ___ 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] [PATCH] POWER8 VSX vectorization libswscale/input.c Track ticket 5570
> Am 23.03.2020 um 12:31 schrieb Pestov Vyacheslav : > > libswscale/ppc/Makefile |3 +- > libswscale/ppc/input_vsx.c| 4562 + > libswscale/swscale.c |2 + > libswscale/swscale_internal.h |1 + > 4 files changed, 4567 insertions(+), 1 deletion(-) > create mode 100644 libswscale/ppc/input_vsx.c Please add some numbers to the commit message that show the typical speedup. Carl Eugen ___ 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 0/2] Pro Pinball Series Soundbank demuxer + decoder.
Hi all, Could I please get some reviews on this? Thanks, Zane On Thu, 19 Mar 2020 12:00:15 + "Zane van Iperen" wrote: > Adds support for the soundbank files used by the Pro Pinball series > of games. > > Please ping for review. > > v3: > - fix potential memory leak if read_header() fails > - fix a buffer overread > - attempt seek before updating state > - remove unneeded check > - naming fixes > > v2: > - Add sanity checks in header fields > - Formatting and comment fixes > - Change the struct names to match the files > > Zane van Iperen (2): > avcodec: add support for Cunning Developments' ADPCM > avformat: add demuxer for Pro Pinball Series' Soundbanks > > Changelog| 2 + > doc/general.texi | 1 + > libavcodec/Makefile | 1 + > libavcodec/adpcm.c | 33 ++ > libavcodec/adpcm_data.c | 13 +++ > libavcodec/adpcm_data.h | 2 + > libavcodec/allcodecs.c | 1 + > libavcodec/avcodec.h | 1 + > libavcodec/codec_desc.c | 7 ++ > libavcodec/version.h | 2 +- > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/pp_bnk.c | 229 > +++ libavformat/version.h| > 2 +- 14 files changed, 294 insertions(+), 2 deletions(-) > create mode 100644 libavformat/pp_bnk.c > > -- > 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". ___ 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] lavc/vaapi_encode: wrap slice codes into row slice functions
Wrap current whole-row slice codes into following functions: - vaapi_encode_make_row_slice() - vaapi_encode_init_row_slice_structure() Signed-off-by: Linjie Fu --- libavcodec/vaapi_encode.c | 184 ++ 1 file changed, 106 insertions(+), 78 deletions(-) diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 8ff720e..8b48379 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -157,6 +157,61 @@ static int vaapi_encode_wait(AVCodecContext *avctx, return 0; } +static int vaapi_encode_make_row_slice(AVCodecContext *avctx, + VAAPIEncodePicture *pic) +{ +VAAPIEncodeContext *ctx = avctx->priv_data; +VAAPIEncodeSlice *slice; +int i, rounding; + +for (i = 0; i < pic->nb_slices; i++) +pic->slices[i].row_size = ctx->slice_size; + +rounding = ctx->slice_block_rows - ctx->nb_slices * ctx->slice_size; +if (rounding > 0) { +// Place rounding error at top and bottom of frame. +av_assert0(rounding < pic->nb_slices); +// Some Intel drivers contain a bug where the encoder will fail +// if the last slice is smaller than the one before it. Since +// that's straightforward to avoid here, just do so. +if (rounding <= 2) { +for (i = 0; i < rounding; i++) +++pic->slices[i].row_size; +} else { +for (i = 0; i < (rounding + 1) / 2; i++) +++pic->slices[pic->nb_slices - i - 1].row_size; +for (i = 0; i < rounding / 2; i++) +++pic->slices[i].row_size; +} +} else if (rounding < 0) { +// Remove rounding error from last slice only. +av_assert0(rounding < ctx->slice_size); +pic->slices[pic->nb_slices - 1].row_size += rounding; +} + +for (i = 0; i < pic->nb_slices; i++) { +slice = &pic->slices[i]; +slice->index = i; +if (i == 0) { +slice->row_start = 0; +slice->block_start = 0; +} else { +const VAAPIEncodeSlice *prev = &pic->slices[i - 1]; +slice->row_start = prev->row_start + prev->row_size; +slice->block_start = prev->block_start + prev->block_size; +} +slice->block_size = slice->row_size * ctx->slice_block_cols; + +av_log(avctx, AV_LOG_DEBUG, "Slice %d: %d-%d (%d rows), " + "%d-%d (%d blocks).\n", i, slice->row_start, + slice->row_start + slice->row_size - 1, slice->row_size, + slice->block_start, slice->block_start + slice->block_size - 1, + slice->block_size); +} + +return 0; +} + static int vaapi_encode_issue(AVCodecContext *avctx, VAAPIEncodePicture *pic) { @@ -340,57 +395,17 @@ static int vaapi_encode_issue(AVCodecContext *avctx, if (pic->nb_slices == 0) pic->nb_slices = ctx->nb_slices; if (pic->nb_slices > 0) { -int rounding; - pic->slices = av_mallocz_array(pic->nb_slices, sizeof(*pic->slices)); if (!pic->slices) { err = AVERROR(ENOMEM); goto fail; } -for (i = 0; i < pic->nb_slices; i++) -pic->slices[i].row_size = ctx->slice_size; - -rounding = ctx->slice_block_rows - ctx->nb_slices * ctx->slice_size; -if (rounding > 0) { -// Place rounding error at top and bottom of frame. -av_assert0(rounding < pic->nb_slices); -// Some Intel drivers contain a bug where the encoder will fail -// if the last slice is smaller than the one before it. Since -// that's straightforward to avoid here, just do so. -if (rounding <= 2) { -for (i = 0; i < rounding; i++) -++pic->slices[i].row_size; -} else { -for (i = 0; i < (rounding + 1) / 2; i++) -++pic->slices[pic->nb_slices - i - 1].row_size; -for (i = 0; i < rounding / 2; i++) -++pic->slices[i].row_size; -} -} else if (rounding < 0) { -// Remove rounding error from last slice only. -av_assert0(rounding < ctx->slice_size); -pic->slices[pic->nb_slices - 1].row_size += rounding; -} +vaapi_encode_make_row_slice(avctx, pic); } + for (i = 0; i < pic->nb_slices; i++) { slice = &pic->slices[i]; -slice->index = i; -if (i == 0) { -slice->row_start = 0; -slice->block_start = 0; -} else { -const VAAPIEncodeSlice *prev = &pic->slices[i - 1]; -slice->row_start = prev->row_start + prev->row_size; -slice->block_start = prev->block_start + prev->block_size; -} -slice->block_size = slice->row_size * ctx->slice_block_cols; - -av_log(avctx, AV_LOG_DEBUG, "Sl
[FFmpeg-devel] [PATCH 2/3] lavc/vaapi_encode: add tile slice encoding support
Add functions to initialize tile slice structure and make tile slice: - vaapi_encode_init_tile_slice_structure - vaapi_encode_make_tile_slice Tile slice is not allowed to cross the boundary of a tile due to the constraints of media-driver. Currently adding support for one slice per tile. N x N tile encoding is supposed to be supported with the the capability of ARBITRARY_MACROBLOCKS slice structures. N X 1 tile encoding should also work in ARBITRARY_ROWS slice structure. Signed-off-by: Linjie Fu --- libavcodec/vaapi_encode.c | 127 +++--- libavcodec/vaapi_encode.h | 16 ++ 2 files changed, 135 insertions(+), 8 deletions(-) diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 8b48379..16b157d 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -212,6 +212,33 @@ static int vaapi_encode_make_row_slice(AVCodecContext *avctx, return 0; } +static int vaapi_encode_make_tile_slice(AVCodecContext *avctx, +VAAPIEncodePicture *pic) +{ +VAAPIEncodeContext *ctx = avctx->priv_data; +VAAPIEncodeSlice *slice; +int i, j, index; + +for (i = 0; i < ctx->tile_cols; i++) { +for (j = 0; j < ctx->tile_rows; j++) { +index= j * ctx->tile_cols + i; +slice= &pic->slices[index]; +slice->index = index; + +pic->slices[index].block_start = ctx->col_bd[i] + + ctx->row_bd[j] * ctx->slice_block_cols; +pic->slices[index].block_size = ctx->row_height[j] * ctx->col_width[i]; + +av_log(avctx, AV_LOG_DEBUG, "Slice %2d: (%2d, %2d) start at: %4d " + "width:%2d height:%2d (%d blocks).\n", index, ctx->col_bd[i], + ctx->row_bd[j], slice->block_start, ctx->col_width[i], + ctx->row_height[j], slice->block_size); +} +} + +return 0; +} + static int vaapi_encode_issue(AVCodecContext *avctx, VAAPIEncodePicture *pic) { @@ -401,7 +428,10 @@ static int vaapi_encode_issue(AVCodecContext *avctx, goto fail; } -vaapi_encode_make_row_slice(avctx, pic); +if (ctx->tile_rows && ctx->tile_cols) +vaapi_encode_make_tile_slice(avctx, pic); +else +vaapi_encode_make_row_slice(avctx, pic); } for (i = 0; i < pic->nb_slices; i++) { @@ -1883,11 +1913,75 @@ static av_cold int vaapi_encode_init_row_slice_structure(AVCodecContext *avctx, return 0; } +static av_cold int vaapi_encode_init_tile_slice_structure(AVCodecContext *avctx, + uint32_t slice_structure) +{ +VAAPIEncodeContext *ctx = avctx->priv_data; +int i, req_tiles; + +if (!(slice_structure & VA_ENC_SLICE_STRUCTURE_ARBITRARY_MACROBLOCKS || + (slice_structure & VA_ENC_SLICE_STRUCTURE_ARBITRARY_ROWS && ctx->tile_cols == 1))) { +av_log(avctx, AV_LOG_ERROR, "Supported slice structure (%#x) doesn't work for " + "current tile requirement.\n", slice_structure); +return AVERROR(EINVAL); +} + +if (ctx->tile_rows > ctx->slice_block_rows || +ctx->tile_cols > ctx->slice_block_cols) { +av_log(avctx, AV_LOG_WARNING, "Not enough block rows/cols (%d x %d) " + "for configured number of tile (%d x %d); ", + ctx->slice_block_rows, ctx->slice_block_cols, + ctx->tile_rows, ctx->tile_cols); +ctx->tile_rows = ctx->tile_rows > ctx->slice_block_rows ? + ctx->slice_block_rows : ctx->tile_rows; +ctx->tile_cols = ctx->tile_cols > ctx->slice_block_cols ? + ctx->slice_block_cols : ctx->tile_cols; +av_log(avctx, AV_LOG_WARNING, "using allowed maximum (%d x %d).\n", + ctx->tile_rows, ctx->tile_cols); +} + +req_tiles = ctx->tile_rows * ctx->tile_cols; + +// Tile slice is not allowed to cross the boundary of a tile due to +// the constraints of media-driver. Currently we support one slice +// per tile. This could be extended to multiple slices per tile. +if (avctx->slices != req_tiles) +av_log(avctx, AV_LOG_WARNING, "The number of requested slices " + "mismatches with configured number of tile (%d != %d); " + "using requested tile number for slice.\n", + avctx->slices, req_tiles); + +ctx->nb_slices = req_tiles; + +// Default in uniform spacing +// 6-3, 6-5 +for (i = 0; i < ctx->tile_cols; i++) { +ctx->col_width[i] = ( i + 1 ) * ctx->slice_block_cols / ctx->tile_cols - +i * ctx->slice_block_cols / ctx->tile_cols; +ctx->col_bd[i + 1] = ctx->col_bd[i] + ctx->col_width[i]; +} +// 6-4, 6-6 +for (i = 0; i < ctx->tile_rows; i++) { +
[FFmpeg-devel] [PATCH 3/3] lavc/vaapi_encode_h265: add h265 tile encoding support
Default to enable uniform_spacing_flag. Guess level by the tile rows/cols. Supported for ICL+ platforms. Also add documentations. To encode with 4 rows 2 columns: ffmpeg ... -c:v hevc_vaapi -tile_rows 4 -tile_cols 2 ... Signed-off-by: Linjie Fu --- doc/encoders.texi | 8 libavcodec/vaapi_encode_h265.c | 36 +++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/doc/encoders.texi b/doc/encoders.texi index e23b6b3..2dc5cd4 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -3091,6 +3091,14 @@ Include HDR metadata if the input frames have it messages). @end table +@item tile_rows +Selects how many rows of tiles to encode with. For example, 4 tile rows would +be requested by setting the tile_rows option to 4. + +@item tile_cols +Selects how many columns of tiles to encode with. For example, 5 tile columns +would be requested by setting the tile_cols option to 5. + @end table @item mjpeg_vaapi diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c index 97dc5a7..457e3d9 100644 --- a/libavcodec/vaapi_encode_h265.c +++ b/libavcodec/vaapi_encode_h265.c @@ -63,6 +63,9 @@ typedef struct VAAPIEncodeH265Context { int level; int sei; +int trows; +int tcols; + // Derived settings. int fixed_qp_idr; int fixed_qp_p; @@ -345,7 +348,7 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) level = ff_h265_guess_level(ptl, avctx->bit_rate, ctx->surface_width, ctx->surface_height, -ctx->nb_slices, 1, 1, +ctx->nb_slices, ctx->tile_rows, ctx->tile_cols, (ctx->b_per_p > 0) + 1); if (level) { av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name); @@ -558,6 +561,20 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) pps->pps_loop_filter_across_slices_enabled_flag = 1; +if (ctx->tile_rows && ctx->tile_cols) { +pps->tiles_enabled_flag = 1; +pps->uniform_spacing_flag = 1; + +pps->num_tile_rows_minus1 = ctx->tile_rows - 1; +pps->num_tile_columns_minus1= ctx->tile_cols - 1; + +pps->loop_filter_across_tiles_enabled_flag = 1; + +for (i = 0; i <= pps->num_tile_rows_minus1; i++) +pps->row_height_minus1[i] = ctx->row_height[i] - 1; +for (i = 0; i <= pps->num_tile_columns_minus1; i++) +pps->column_width_minus1[i] = ctx->col_width[i] - 1; +} // Fill VAAPI parameter buffers. @@ -666,6 +683,13 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) }, }; +if (pps->tiles_enabled_flag) { +for (i = 0; i <= vpic->num_tile_rows_minus1; i++) +vpic->row_height_minus1[i] = pps->row_height_minus1[i]; +for (i = 0; i <= vpic->num_tile_columns_minus1; i++) +vpic->column_width_minus1[i] = pps->column_width_minus1[i]; +} + return 0; } @@ -1181,6 +1205,11 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext *avctx) if (priv->qp > 0) ctx->explicit_qp = priv->qp; +if (priv->trows && priv->tcols) { +ctx->tile_rows = priv->trows; +ctx->tile_cols = priv->tcols; +} + return ff_vaapi_encode_init(avctx); } @@ -1257,6 +1286,11 @@ static const AVOption vaapi_encode_h265_options[] = { { .i64 = SEI_MASTERING_DISPLAY | SEI_CONTENT_LIGHT_LEVEL }, INT_MIN, INT_MAX, FLAGS, "sei" }, +{ "tile_rows", "Number of rows for tile encoding", + OFFSET(trows), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS }, +{ "tile_cols", "Number of cols for tile encoding", + OFFSET(tcols), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS }, + { NULL }, }; -- 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 3/3] lavc/vaapi_encode_h265: add h265 tile encoding support
On Mon, Mar 23, 2020 at 10:30 PM Linjie Fu wrote: > > Default to enable uniform_spacing_flag. Guess level by the tile > rows/cols. Supported for ICL+ platforms. > > Also add documentations. > > To encode with 4 rows 2 columns: > ffmpeg ... -c:v hevc_vaapi -tile_rows 4 -tile_cols 2 ... > > Signed-off-by: Linjie Fu > --- > doc/encoders.texi | 8 > libavcodec/vaapi_encode_h265.c | 36 +++- > 2 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/doc/encoders.texi b/doc/encoders.texi > index e23b6b3..2dc5cd4 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -3091,6 +3091,14 @@ Include HDR metadata if the input frames have it > messages). > @end table > > +@item tile_rows > +Selects how many rows of tiles to encode with. For example, 4 tile rows would > +be requested by setting the tile_rows option to 4. > + > +@item tile_cols > +Selects how many columns of tiles to encode with. For example, 5 tile columns > +would be requested by setting the tile_cols option to 5. > + > @end table > > @item mjpeg_vaapi > diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c > index 97dc5a7..457e3d9 100644 > --- a/libavcodec/vaapi_encode_h265.c > +++ b/libavcodec/vaapi_encode_h265.c > @@ -63,6 +63,9 @@ typedef struct VAAPIEncodeH265Context { > int level; > int sei; > > +int trows; > +int tcols; > + > // Derived settings. > int fixed_qp_idr; > int fixed_qp_p; > @@ -345,7 +348,7 @@ static int > vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) > > level = ff_h265_guess_level(ptl, avctx->bit_rate, > ctx->surface_width, ctx->surface_height, > -ctx->nb_slices, 1, 1, > +ctx->nb_slices, ctx->tile_rows, > ctx->tile_cols, > (ctx->b_per_p > 0) + 1); > if (level) { > av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name); > @@ -558,6 +561,20 @@ static int > vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) > > pps->pps_loop_filter_across_slices_enabled_flag = 1; > > +if (ctx->tile_rows && ctx->tile_cols) { > +pps->tiles_enabled_flag = 1; > +pps->uniform_spacing_flag = 1; > + > +pps->num_tile_rows_minus1 = ctx->tile_rows - 1; > +pps->num_tile_columns_minus1= ctx->tile_cols - 1; > + > +pps->loop_filter_across_tiles_enabled_flag = 1; > + > +for (i = 0; i <= pps->num_tile_rows_minus1; i++) > +pps->row_height_minus1[i] = ctx->row_height[i] - 1; > +for (i = 0; i <= pps->num_tile_columns_minus1; i++) > +pps->column_width_minus1[i] = ctx->col_width[i] - 1; > +} > > // Fill VAAPI parameter buffers. > > @@ -666,6 +683,13 @@ static int > vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) > }, > }; > > +if (pps->tiles_enabled_flag) { > +for (i = 0; i <= vpic->num_tile_rows_minus1; i++) > +vpic->row_height_minus1[i] = pps->row_height_minus1[i]; > +for (i = 0; i <= vpic->num_tile_columns_minus1; i++) > +vpic->column_width_minus1[i] = pps->column_width_minus1[i]; > +} > + > return 0; > } > > @@ -1181,6 +1205,11 @@ static av_cold int > vaapi_encode_h265_init(AVCodecContext *avctx) > if (priv->qp > 0) > ctx->explicit_qp = priv->qp; > > +if (priv->trows && priv->tcols) { > +ctx->tile_rows = priv->trows; > +ctx->tile_cols = priv->tcols; > +} > + > return ff_vaapi_encode_init(avctx); > } > > @@ -1257,6 +1286,11 @@ static const AVOption vaapi_encode_h265_options[] = { >{ .i64 = SEI_MASTERING_DISPLAY | SEI_CONTENT_LIGHT_LEVEL }, >INT_MIN, INT_MAX, FLAGS, "sei" }, > > +{ "tile_rows", "Number of rows for tile encoding", > + OFFSET(trows), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS }, > +{ "tile_cols", "Number of cols for tile encoding", > + OFFSET(tcols), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS }, > + > { NULL }, > }; > > -- As my viewpoint, -tiles 3x2 (rows x cols) more naturally ___ 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 13/18] h264_sei: parse the picture timing SEIs correctly
Quoting Kieran Kunhya (2020-03-22 05:24:35) > On Fri, 13 Mar 2020 at 10:32, Anton Khirnov wrote: > > > Those SEIs refer to the currently active SPS. However, since the SEI > > NALUs precede the coded picture data in the bitstream, the active SPS is > > in general not known when we are decoding the SEI. > > > > The spec disagrees with this statement (7.4.1.2.1 Order of sequence and > picture parameter set RBSPs and their activation). There are reasonably > clear rules about SPS activation. > (Whether real world bitstreams comply is a different issue of course). I don't think you read that section carefully enough. What it says (in my interpretation) is that an SPS can be activated only by: - buffering period SEI - a PPS (which is itself activated by picture data) Picture timing SEI is neither of those. Also see D.2.2 Note 1, which suggests exactly what I'm doing with this patch. -- Anton Khirnov ___ 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] avcodec/cbs_h264_syntax_template: fix off by 1 error with slice_group_change_cycle
Fixes: assertion failure Fixes: 20390/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5683400772157440 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/cbs_h264_syntax_template.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c index 878d348b94..b65460996b 100644 --- a/libavcodec/cbs_h264_syntax_template.c +++ b/libavcodec/cbs_h264_syntax_template.c @@ -1366,7 +1366,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, (sps->pic_height_in_map_units_minus1 + 1); max = (pic_size + pps->slice_group_change_rate_minus1) / (pps->slice_group_change_rate_minus1 + 1); -bits = av_log2(2 * max - 1); +bits = av_ceil_log2(max + 1); u(bits, slice_group_change_cycle, 0, max); } -- 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 13/18] h264_sei: parse the picture timing SEIs correctly
On 3/23/2020 1:17 PM, Anton Khirnov wrote: > Quoting Kieran Kunhya (2020-03-22 05:24:35) >> On Fri, 13 Mar 2020 at 10:32, Anton Khirnov wrote: >> >>> Those SEIs refer to the currently active SPS. However, since the SEI >>> NALUs precede the coded picture data in the bitstream, the active SPS is >>> in general not known when we are decoding the SEI. >>> >> >> The spec disagrees with this statement (7.4.1.2.1 Order of sequence and >> picture parameter set RBSPs and their activation). There are reasonably >> clear rules about SPS activation. >> (Whether real world bitstreams comply is a different issue of course). > > I don't think you read that section carefully enough. What it says (in > my interpretation) is that an SPS can be activated only by: > - buffering period SEI Looking at our Buffering Period SEI parsing code, it doesn't look like we're activating the SPS it references, for that matter. > - a PPS (which is itself activated by picture data) > Picture timing SEI is neither of those. > > Also see D.2.2 Note 1, which suggests exactly what I'm doing with this > patch. > ___ 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] h264_sei: parse the picture timing SEIs correctly
Those SEIs refer to the currently active SPS. However, since the SEI NALUs precede the coded picture data in the bitstream, the active SPS is in general not known when we are decoding the SEI. Therefore, store the content of the picture timing SEIs and actually parse it when the active SPS is known. --- Thank you, stupid bug found and fixed by simplifying the code. --- libavcodec/h264_parser.c | 9 + libavcodec/h264_sei.c| 82 +++- libavcodec/h264_sei.h| 11 ++ libavcodec/h264_slice.c | 10 + 4 files changed, 78 insertions(+), 34 deletions(-) diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 5f9a9c46ef..ec1cbc6a66 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -481,6 +481,15 @@ static inline int parse_nal_units(AVCodecParserContext *s, } } +if (p->sei.picture_timing.present) { +ret = ff_h264_sei_process_picture_timing(&p->sei.picture_timing, + sps, avctx); +if (ret < 0) { +av_log(avctx, AV_LOG_ERROR, "Error processing the picture timing SEI\n"); +p->sei.picture_timing.present = 0; +} +} + if (sps->pic_struct_present_flag && p->sei.picture_timing.present) { switch (p->sei.picture_timing.pic_struct) { case H264_SEI_PIC_STRUCT_TOP_FIELD: diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c index 32d13985f3..870dd90717 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -54,30 +54,22 @@ void ff_h264_sei_uninit(H264SEIContext *h) av_buffer_unref(&h->a53_caption.buf_ref); } -static int decode_picture_timing(H264SEIPictureTiming *h, GetBitContext *gb, - const H264ParamSets *ps, void *logctx) +int ff_h264_sei_process_picture_timing(H264SEIPictureTiming *h, const SPS *sps, + void *logctx) { -int i; -const SPS *sps = ps->sps; - -for (i = 0; ilog2_max_frame_num) && ps->sps_list[i]) -sps = (const SPS *)ps->sps_list[i]->data; +GetBitContext gb; -if (!sps) { -av_log(logctx, AV_LOG_ERROR, "SPS unavailable in decode_picture_timing\n"); -return AVERROR_PS_NOT_FOUND; -} +init_get_bits(&gb, h->payload, h->payload_size_bits); if (sps->nal_hrd_parameters_present_flag || sps->vcl_hrd_parameters_present_flag) { -h->cpb_removal_delay = get_bits_long(gb, sps->cpb_removal_delay_length); -h->dpb_output_delay = get_bits_long(gb, sps->dpb_output_delay_length); +h->cpb_removal_delay = get_bits_long(&gb, sps->cpb_removal_delay_length); +h->dpb_output_delay = get_bits_long(&gb, sps->dpb_output_delay_length); } if (sps->pic_struct_present_flag) { unsigned int i, num_clock_ts; -h->pic_struct = get_bits(gb, 4); +h->pic_struct = get_bits(&gb, 4); h->ct_type= 0; if (h->pic_struct > H264_SEI_PIC_STRUCT_FRAME_TRIPLING) @@ -86,38 +78,38 @@ static int decode_picture_timing(H264SEIPictureTiming *h, GetBitContext *gb, num_clock_ts = sei_num_clock_ts_table[h->pic_struct]; h->timecode_cnt = 0; for (i = 0; i < num_clock_ts; i++) { -if (get_bits(gb, 1)) { /* clock_timestamp_flag */ +if (get_bits(&gb, 1)) { /* clock_timestamp_flag */ H264SEITimeCode *tc = &h->timecode[h->timecode_cnt++]; unsigned int full_timestamp_flag; unsigned int counting_type, cnt_dropped_flag; -h->ct_type |= 1 << get_bits(gb, 2); -skip_bits(gb, 1); /* nuit_field_based_flag */ -counting_type = get_bits(gb, 5);/* counting_type */ -full_timestamp_flag = get_bits(gb, 1); -skip_bits(gb, 1); /* discontinuity_flag */ -cnt_dropped_flag = get_bits(gb, 1); /* cnt_dropped_flag */ +h->ct_type |= 1 << get_bits(&gb, 2); +skip_bits(&gb, 1); /* nuit_field_based_flag */ +counting_type = get_bits(&gb, 5);/* counting_type */ +full_timestamp_flag = get_bits(&gb, 1); +skip_bits(&gb, 1); /* discontinuity_flag */ +cnt_dropped_flag = get_bits(&gb, 1); /* cnt_dropped_flag */ if (cnt_dropped_flag && counting_type > 1 && counting_type < 7) tc->dropframe = 1; -tc->frame = get_bits(gb, 8); /* n_frames */ +tc->frame = get_bits(&gb, 8); /* n_frames */ if (full_timestamp_flag) { tc->full = 1; -tc->seconds
Re: [FFmpeg-devel] [PATCH 13/18] h264_sei: parse the picture timing SEIs correctly
Quoting James Almer (2020-03-23 17:20:56) > On 3/23/2020 1:17 PM, Anton Khirnov wrote: > > Quoting Kieran Kunhya (2020-03-22 05:24:35) > >> On Fri, 13 Mar 2020 at 10:32, Anton Khirnov wrote: > >> > >>> Those SEIs refer to the currently active SPS. However, since the SEI > >>> NALUs precede the coded picture data in the bitstream, the active SPS is > >>> in general not known when we are decoding the SEI. > >>> > >> > >> The spec disagrees with this statement (7.4.1.2.1 Order of sequence and > >> picture parameter set RBSPs and their activation). There are reasonably > >> clear rules about SPS activation. > >> (Whether real world bitstreams comply is a different issue of course). > > > > I don't think you read that section carefully enough. What it says (in > > my interpretation) is that an SPS can be activated only by: > > - buffering period SEI > > Looking at our Buffering Period SEI parsing code, it doesn't look like > we're activating the SPS it references, for that matter. Right, but then against we're not really using it for anything so it probably doesn't matter. -- Anton Khirnov ___ 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] avcodec/wavpack: Prevent frame format from being wrong
Quoting Michael Niedermayer (2020-03-20 21:50:18) > On Fri, Mar 20, 2020 at 10:18:49AM +0100, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2020-03-20 01:03:36) > > > Fixes: out of array access > > > Fixes: > > > 21193/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVPACK_fuzzer-5125168956702720 > > > > > > Found-by: continuous fuzzing process > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer > > > --- > > > libavcodec/wavpack.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c > > > index b27262b94e..e9c870e41e 100644 > > > --- a/libavcodec/wavpack.c > > > +++ b/libavcodec/wavpack.c > > > @@ -1488,6 +1488,7 @@ static int wavpack_decode_block(AVCodecContext > > > *avctx, int block_no, > > > > > > /* get output buffer */ > > > wc->curr_frame.f->nb_samples = s->samples; > > > +wc->curr_frame.f->format = avctx->sample_fmt; > > > > How does this have any effect? curr_frame.f should now be clean and get > > initialized from avctx->sample_fmt. > > IIRC > The format changes between frames, so the struct is still set to the one > from the previous frame and that overrides the use of the avctx value > > setting it to NONE (here or somewhere else) should work too. ff_thread_release_buffer() is called on that frame immediately before, which should reset it to defaults (setting format to FMT_NONE). -- Anton Khirnov ___ 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] JPEG2000 Decoder/Encoder Enhancements
On Sun, Mar 22, 2020 at 06:58:24PM +0100, Carl Eugen Hoyos wrote: > Am So., 22. März 2020 um 18:27 Uhr schrieb Gautam Ramakrishnan > : > > > I saw the list of unmentored projects for GSoC and enhancements to the > > JPEG2000 decoder and encoder were there. I am finding it very > > difficult to make a proposal as the specifications are not available > > for free. However, I am willing to work on improving the JPEG2000 > > features even if it is not through GSoC if someone can guide me > > through it. > > > > I will basically need some guidance on the specifications and some > > advice on how to go about implementing the missing features. Could I > > take this up even if it is not a part of GSoC? > > Google can find an (old but probably usable) version of the standard. > > You can only work on this task if you find a mentor;-( i can help mentoring a jpeg2000 related project but the standard is a bit convoluted. And while i do know jpeg2000 a bit iam no jpeg2000 expert. So for many detail questions i would have to spend considerable time looking through the specification. I dont have the time for doing that alot So if Gautam can wrap his head around the specification and has the time to decipher the details where needed. Meaning if the majority of the time spend is on Gautam or whoever does the work then i can (help) mentor it probably Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates signature.asc Description: PGP signature ___ 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 18/18] h264_ps: pass AVCodecContext as void* where possible
On Wed, Mar 18, 2020 at 10:02:39AM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2020-03-16 12:23:11) > > On Sun, Mar 15, 2020 at 06:02:04PM +0100, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2020-03-13 23:29:12) > > > > On Fri, Mar 13, 2020 at 11:28:50AM +0100, Anton Khirnov wrote: > > > > > Makes sure it is only used for logging and nothing else. > > > > > --- > > > > > libavcodec/h264_ps.c | 18 +- > > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c > > > > > index 1951bb1161..4ef25aa514 100644 > > > > > --- a/libavcodec/h264_ps.c > > > > > +++ b/libavcodec/h264_ps.c > > > > > @@ -104,14 +104,14 @@ static void remove_sps(H264ParamSets *s, int id) > > > > > av_buffer_unref(&s->sps_list[id]); > > > > > } > > > > > > > > > > -static inline int decode_hrd_parameters(GetBitContext *gb, > > > > > AVCodecContext *avctx, > > > > > +static inline int decode_hrd_parameters(GetBitContext *gb, void > > > > > *logctx, > > > > > > > > this is a double sided sword > > > > while fields of logctx cannot be used its after this possible to pass > > > > wrong things as logctx > > > > > > Right, but that should be easily noticeable since it will crash on > > > dereferencing the AVClass. I consider the danger of people accessing the > > > AVCodecContext inappropriately to be bigger (since it's done in many > > > places already). > > > > > > > > But we might want to consider something like > > > typedef AVClass* AVLogger > > > > yes, if that ends up looking clean in practice then iam certainly in favor > > That would require changes to the logging API though, so would be > outside of the scope of this set. > Are you objecting to the patch as it is? no i dont really mind, iam fine with the code as it is before as well as after the patch. Its better in one way worse in another. So really no reason for me to be against this if someone else has a preferrance Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: PGP signature ___ 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 14/18] h264_ps: make the PPS hold a reference to its SPS
Quoting James Almer (2020-03-18 17:05:38) > On 3/13/2020 7:28 AM, Anton Khirnov wrote: > > It represents the relationship between them more naturally and will be > > useful in the following commits. > > > > Allows significantly more frames in fate-h264-attachment-631 to be > > decoded. > > That sample is odd. It has an SPS/PPS pair in extradata that's wrong for > the Buffering Period and Picture Timing SEI messages in-band, but > seemingly correct to actually decode all these frames revealed by this > patch. > The SPS that makes the aforementioned SEI messages parseable, but the > frames seemingly undecodeable, is in-band in the second RAP (Which fwiw > reports a different bitstream level). So this patch prevents the > extradata PPS from using the in-band SPS after it replaced the extradata > one in sps_list. There's no PPS in-band. > > Is this really intended? Is the active PPS' sps_id meant to reference > the SPS with that id that's currently "valid" and in sps_list, or the > whichever one was valid at the time the PPS was first parsed?. If the > latter, then why replace the SPS in-band but never the PPS? > The in-band SPS is virtually unused after this change. My guess would be that someone (or some/thing/, dun dun dun) did some botched/misunderstood processing on that sample, so trying to figure out what was intended is futile. According to the spec, SPS contents must not change within a sequence (they can change on sequence boundaries), so this sample is invalid. From a more practical viewpoint, since a PPS is parsed for a given SPS I'd say it is clearer, cleaner and safer to tie them together like I'm doing in this patch. > > > --- > > libavcodec/h264_ps.c | 29 +- > > libavcodec/h264_ps.h | 3 + > > libavcodec/h264_slice.c| 14 +-- > > Missing changes to h264_parser.c, and one "h->ps.sps == (const > SPS*)h->ps.sps_list[h->ps.pps->sps_id]->data" check in h264dec.c Good catch, fixed locally. -- Anton Khirnov ___ 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 1/2] avcodec: add support for Cunning Developments' ADPCM
On Thu, Mar 19, 2020 at 12:00:20PM +, Zane van Iperen wrote: [...] > index 4cce0a5857..a1f6966c75 100644 > --- a/libavcodec/adpcm_data.c > +++ b/libavcodec/adpcm_data.c > @@ -177,3 +177,16 @@ const int16_t ff_adpcm_mtaf_stepsize[32][16] = { > { 424, 1273, 2121, 2970, 3819, 4668, 5516, 6365, > -424, -1273, -2121, -2970, -3819, -4668, -5516, -6365, }, > }; > + > +const int16_t ff_adpcm_ima_cunning_index_table[8] = { > +-1, -1, -1, -1, 1, 2, 3, 4, > +}; this could be int8_t [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato signature.asc Description: PGP signature ___ 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 14/18] h264_ps: make the PPS hold a reference to its SPS
On 3/23/2020 2:36 PM, Anton Khirnov wrote: > Quoting James Almer (2020-03-18 17:05:38) >> On 3/13/2020 7:28 AM, Anton Khirnov wrote: >>> It represents the relationship between them more naturally and will be >>> useful in the following commits. >>> >>> Allows significantly more frames in fate-h264-attachment-631 to be >>> decoded. >> >> That sample is odd. It has an SPS/PPS pair in extradata that's wrong for >> the Buffering Period and Picture Timing SEI messages in-band, but >> seemingly correct to actually decode all these frames revealed by this >> patch. >> The SPS that makes the aforementioned SEI messages parseable, but the >> frames seemingly undecodeable, is in-band in the second RAP (Which fwiw >> reports a different bitstream level). So this patch prevents the >> extradata PPS from using the in-band SPS after it replaced the extradata >> one in sps_list. There's no PPS in-band. >> >> Is this really intended? Is the active PPS' sps_id meant to reference >> the SPS with that id that's currently "valid" and in sps_list, or the >> whichever one was valid at the time the PPS was first parsed?. If the >> latter, then why replace the SPS in-band but never the PPS? >> The in-band SPS is virtually unused after this change. > > My guess would be that someone (or some/thing/, dun dun dun) did some > botched/misunderstood processing on that sample, so trying to figure out > what was intended is futile. I just find it bizarre that the extradata SPS makes the frames decode and the SEI unparseable, but the in-band SPS makes the SEI parseable and the frames undecodeable. > > According to the spec, SPS contents must not change within a sequence > (they can change on sequence boundaries), so this sample is invalid. > From a more practical viewpoint, since a PPS is parsed for a given SPS > I'd say it is clearer, cleaner and safer to tie them together like I'm > doing in this patch. There's commented out code in remove_sps() that used to drop all PPS that depended on a given SPS once a new one with the same id was found. I'm going to guess it was done to keep this mess of a sample working. Since now you're making this behavior official, you could remove that dead code while at it. > >> >>> --- >>> libavcodec/h264_ps.c | 29 +- >>> libavcodec/h264_ps.h | 3 + >>> libavcodec/h264_slice.c| 14 +-- >> >> Missing changes to h264_parser.c, and one "h->ps.sps == (const >> SPS*)h->ps.sps_list[h->ps.pps->sps_id]->data" check in h264dec.c > > Good catch, fixed locally. LGTM then. ___ 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 v1] avformat/mxfdec: use av_asprintf()
sön 2020-03-22 klockan 23:03 +0800 skrev lance.lmw...@gmail.com: > From: Limin Wang > > Signed-off-by: Limin Wang > --- > libavformat/mxfdec.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index 9113e2a09c..3374f36a88 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -2017,7 +2017,7 @@ static MXFStructuralComponent* > mxf_resolve_sourceclip(MXFContext *mxf, UID *stro > static int mxf_parse_package_comments(MXFContext *mxf, AVDictionary > **pm, MXFPackage *package) > { > MXFTaggedValue *tag; > -int size, i; > +int i; > char *key = NULL; > > for (i = 0; i < package->comment_count; i++) { > @@ -2025,12 +2025,10 @@ static int > mxf_parse_package_comments(MXFContext *mxf, AVDictionary **pm, MXFPac > if (!tag || !tag->name || !tag->value) > continue; > > -size = strlen(tag->name) + 8 + 1; > -key = av_mallocz(size); > +key = av_asprintf("comment_%s", tag->name); > if (!key) > return AVERROR(ENOMEM); > > -snprintf(key, size, "comment_%s", tag->name); > av_dict_set(pm, key, tag->value, AV_DICT_DONT_STRDUP_KEY); > } > return 0; Looks OK /Tomas ___ 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] MAINTAINERS: add my gpg fingerprint
On Mon, Mar 23, 2020 at 04:11:04AM +0100, Ramiro Polla wrote: > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index f9810d5594..9238a1a762 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -614,6 +614,7 @@ Nikolay Aleksandrov 8978 1D8C FB71 588E 4B27 > EAA8 C4F0 B5FC E011 13B1 > Panagiotis Issaris6571 13A3 33D9 3726 F728 AA98 F643 B12E ECF3 > E029 > Peter RossA907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 > DD6B > Philip Langdale 5DC5 8D66 5FBA 3A43 18EC 045E F8D6 B194 6A75 > 682E > +Ramiro Polla 1E0D 3820 ACCB 36AF 97B4 F18C 648E 2B0A E905 > E26A > Reimar Doeffinger C61D 16E5 9E2C D10C 8958 38A4 0899 A2B9 06D4 > D9C7 > Reinhard Tartler 9300 5DC2 7E87 6C37 ED7B CA9A 9808 3544 9453 > 48A4 > Reynaldo H. Verdejo Pinochet 6E27 CD34 170C C78E 4D4F 5F40 C18E 077F 3114 > 452A iam unable to find a matching key on the keyserver gpg --search-key "ramiro polla" gpg: searching for "ramiro polla" from hkp server keys.gnupg.net (1) Ramiro Polla 2048 bit RSA key 9B6C5700, created: 2014-09-23 (2) Ramiro Polla 1024 bit DSA key 25E635F9, created: 2010-01-08 pub 2048R/9B6C5700 2014-09-23 Key fingerprint = 7859 C65B 751B 1179 792E DAE8 8E95 8B2F 9B6C 5700 uid Ramiro Polla sub 2048R/CAF28B6D 2014-09-23 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If the United States is serious about tackling the national security threats related to an insecure 5G network, it needs to rethink the extent to which it values corporate profits and government espionage over security.-Bruce Schneier signature.asc Description: PGP signature ___ 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] MAINTAINERS: add my gpg fingerprint
Hi Michael, On Mon, Mar 23, 2020 at 8:44 PM Michael Niedermayer wrote: > > On Mon, Mar 23, 2020 at 04:11:04AM +0100, Ramiro Polla wrote: > > --- > > MAINTAINERS | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f9810d5594..9238a1a762 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -614,6 +614,7 @@ Nikolay Aleksandrov 8978 1D8C FB71 588E 4B27 > > EAA8 C4F0 B5FC E011 13B1 > > Panagiotis Issaris6571 13A3 33D9 3726 F728 AA98 F643 B12E ECF3 > > E029 > > Peter RossA907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 > > DD6B > > Philip Langdale 5DC5 8D66 5FBA 3A43 18EC 045E F8D6 B194 6A75 > > 682E > > +Ramiro Polla 1E0D 3820 ACCB 36AF 97B4 F18C 648E 2B0A E905 > > E26A > > Reimar Doeffinger C61D 16E5 9E2C D10C 8958 38A4 0899 A2B9 06D4 > > D9C7 > > Reinhard Tartler 9300 5DC2 7E87 6C37 ED7B CA9A 9808 3544 9453 > > 48A4 > > Reynaldo H. Verdejo Pinochet 6E27 CD34 170C C78E 4D4F 5F40 C18E 077F 3114 > > 452A > > iam unable to find a matching key on the keyserver > > gpg --search-key "ramiro polla" > gpg: searching for "ramiro polla" from hkp server keys.gnupg.net > (1) Ramiro Polla > 2048 bit RSA key 9B6C5700, created: 2014-09-23 > (2) Ramiro Polla > 1024 bit DSA key 25E635F9, created: 2010-01-08 > pub 2048R/9B6C5700 2014-09-23 > Key fingerprint = 7859 C65B 751B 1179 792E DAE8 8E95 8B2F 9B6C 5700 > uid Ramiro Polla > sub 2048R/CAF28B6D 2014-09-23 Sorry, I have very little experience with this. That's an old key, but I found it now. New patch attached with the 2014 fingerprint (also attached the patch signed with that key just because why not...). Hopefully this is good now. 0001-MAINTAINERS-add-my-gpg-fingerprint.patch.sig Description: PGP signature From 3c1603bd0c698fc9957c5bda718c176e6084ee2c Mon Sep 17 00:00:00 2001 From: Ramiro Polla Date: Mon, 23 Mar 2020 04:02:25 +0100 Subject: [PATCH] MAINTAINERS: add my gpg fingerprint --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index f9810d5594..e19d1ee586 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -614,6 +614,7 @@ Nikolay Aleksandrov 8978 1D8C FB71 588E 4B27 EAA8 C4F0 B5FC E011 13B1 Panagiotis Issaris6571 13A3 33D9 3726 F728 AA98 F643 B12E ECF3 E029 Peter RossA907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B Philip Langdale 5DC5 8D66 5FBA 3A43 18EC 045E F8D6 B194 6A75 682E +Ramiro Polla 7859 C65B 751B 1179 792E DAE8 8E95 8B2F 9B6C 5700 Reimar Doeffinger C61D 16E5 9E2C D10C 8958 38A4 0899 A2B9 06D4 D9C7 Reinhard Tartler 9300 5DC2 7E87 6C37 ED7B CA9A 9808 3544 9453 48A4 Reynaldo H. Verdejo Pinochet 6E27 CD34 170C C78E 4D4F 5F40 C18E 077F 3114 452A -- 2.11.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 13/18] h264_sei: parse the picture timing SEIs correctly
> > I don't think you read that section carefully enough. What it says (in > my interpretation) is that an SPS can be activated only by: > - buffering period SEI > - a PPS (which is itself activated by picture data) > Picture timing SEI is neither of those. > The contents of pic-struct do not affect activation but have dependent syntax elements. So for example a Buffering Period SEI could activate an SPS and then followed by pic-struct and pic struct would then have a dependency on the activated SPS. This SPS could be different to the one the VCL data refers to. I think there are other corner cases as well. Kieran ___ 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] avcodec/motion_est: fix penalty_factor for b frames
On Sun, Mar 22, 2020 at 04:55:25PM +0100, Ramiro Polla wrote: > In ff_estimate_b_frame_motion(), penalty_factor would be used before > being initialized in estimate_motion_b(). Also, the initialization > would happen more than once unnecessarily. > --- > libavcodec/motion_est.c | 15 --- > tests/ref/vsynth/vsynth2-mpeg2-422 | 6 +++--- > tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd | 6 +++--- > tests/ref/vsynth/vsynth2-mpeg4-adap | 6 +++--- > 4 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c > index 02c75fd470..1feb46cec3 100644 > --- a/libavcodec/motion_est.c > +++ b/libavcodec/motion_est.c > @@ -1123,9 +1123,6 @@ static int estimate_motion_b(MpegEncContext *s, int > mb_x, int mb_y, > uint8_t * const mv_penalty= c->mv_penalty[f_code] + MAX_DMV; > int mv_scale; > > -c->penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > c->avctx->me_cmp); > -c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > c->avctx->me_sub_cmp); > -c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, > c->avctx->mb_cmp); > c->current_mv_penalty= mv_penalty; > > get_limits(s, 16*mb_x, 16*mb_y); > @@ -1491,7 +1488,6 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > int mb_x, int mb_y) > { > MotionEstContext * const c= &s->me; > -const int penalty_factor= c->mb_penalty_factor; > int fmin, bmin, dmin, fbmin, bimin, fimin; > int type=0; > const int xy = mb_y*s->mb_stride + mb_x; > @@ -1517,18 +1513,23 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > dmin= direct_search(s, mb_x, mb_y); > else > dmin= INT_MAX; > + > +c->penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > c->avctx->me_cmp); > +c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > c->avctx->me_sub_cmp); > +c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, > c->avctx->mb_cmp); If mb_penalty_factor isnt correct in this before this maybe isnt enough as the direct_search() uses mb_penalty_factor thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: PGP signature ___ 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] MAINTAINERS: add my gpg fingerprint
On Mon, Mar 23, 2020 at 09:57:31PM +0100, Ramiro Polla wrote: > Hi Michael, > > On Mon, Mar 23, 2020 at 8:44 PM Michael Niedermayer > wrote: > > > > On Mon, Mar 23, 2020 at 04:11:04AM +0100, Ramiro Polla wrote: > > > --- > > > MAINTAINERS | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index f9810d5594..9238a1a762 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -614,6 +614,7 @@ Nikolay Aleksandrov 8978 1D8C FB71 588E > > > 4B27 EAA8 C4F0 B5FC E011 13B1 > > > Panagiotis Issaris6571 13A3 33D9 3726 F728 AA98 F643 B12E > > > ECF3 E029 > > > Peter RossA907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 > > > AC40 DD6B > > > Philip Langdale 5DC5 8D66 5FBA 3A43 18EC 045E F8D6 B194 > > > 6A75 682E > > > +Ramiro Polla 1E0D 3820 ACCB 36AF 97B4 F18C 648E 2B0A > > > E905 E26A > > > Reimar Doeffinger C61D 16E5 9E2C D10C 8958 38A4 0899 A2B9 > > > 06D4 D9C7 > > > Reinhard Tartler 9300 5DC2 7E87 6C37 ED7B CA9A 9808 3544 > > > 9453 48A4 > > > Reynaldo H. Verdejo Pinochet 6E27 CD34 170C C78E 4D4F 5F40 C18E 077F > > > 3114 452A > > > > iam unable to find a matching key on the keyserver > > > > gpg --search-key "ramiro polla" > > gpg: searching for "ramiro polla" from hkp server keys.gnupg.net > > (1) Ramiro Polla > > 2048 bit RSA key 9B6C5700, created: 2014-09-23 > > (2) Ramiro Polla > > 1024 bit DSA key 25E635F9, created: 2010-01-08 > > > pub 2048R/9B6C5700 2014-09-23 > > Key fingerprint = 7859 C65B 751B 1179 792E DAE8 8E95 8B2F 9B6C 5700 > > uid Ramiro Polla > > sub 2048R/CAF28B6D 2014-09-23 > > Sorry, I have very little experience with this. That's an old key, but > I found it now. > > New patch attached with the 2014 fingerprint (also attached the patch > signed with that key just because why not...). > > Hopefully this is good now. sure, will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: PGP signature ___ 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 v1] avformat/mxfdec: use av_asprintf()
On Mon, Mar 23, 2020 at 08:29:32PM +0100, Tomas Härdin wrote: > sön 2020-03-22 klockan 23:03 +0800 skrev lance.lmw...@gmail.com: > > From: Limin Wang > > > > Signed-off-by: Limin Wang > > --- > > libavformat/mxfdec.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index 9113e2a09c..3374f36a88 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -2017,7 +2017,7 @@ static MXFStructuralComponent* > > mxf_resolve_sourceclip(MXFContext *mxf, UID *stro > > static int mxf_parse_package_comments(MXFContext *mxf, AVDictionary > > **pm, MXFPackage *package) > > { > > MXFTaggedValue *tag; > > -int size, i; > > +int i; > > char *key = NULL; > > > > for (i = 0; i < package->comment_count; i++) { > > @@ -2025,12 +2025,10 @@ static int > > mxf_parse_package_comments(MXFContext *mxf, AVDictionary **pm, MXFPac > > if (!tag || !tag->name || !tag->value) > > continue; > > > > -size = strlen(tag->name) + 8 + 1; > > -key = av_mallocz(size); > > +key = av_asprintf("comment_%s", tag->name); > > if (!key) > > return AVERROR(ENOMEM); > > > > -snprintf(key, size, "comment_%s", tag->name); > > av_dict_set(pm, key, tag->value, AV_DICT_DONT_STRDUP_KEY); > > } > > return 0; > > Looks OK will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "Nothing to hide" only works if the folks in power share the values of you and everyone you know entirely and always will -- Tom Scott signature.asc Description: PGP signature ___ 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 14/18] h264_ps: make the PPS hold a reference to its SPS
Anton Khirnov: > Quoting James Almer (2020-03-18 17:05:38) >> On 3/13/2020 7:28 AM, Anton Khirnov wrote: >>> It represents the relationship between them more naturally and will be >>> useful in the following commits. >>> >>> Allows significantly more frames in fate-h264-attachment-631 to be >>> decoded. >> >> That sample is odd. It has an SPS/PPS pair in extradata that's wrong for >> the Buffering Period and Picture Timing SEI messages in-band, but >> seemingly correct to actually decode all these frames revealed by this >> patch. >> The SPS that makes the aforementioned SEI messages parseable, but the >> frames seemingly undecodeable, is in-band in the second RAP (Which fwiw >> reports a different bitstream level). So this patch prevents the >> extradata PPS from using the in-band SPS after it replaced the extradata >> one in sps_list. There's no PPS in-band. >> >> Is this really intended? Is the active PPS' sps_id meant to reference >> the SPS with that id that's currently "valid" and in sps_list, or the >> whichever one was valid at the time the PPS was first parsed?. If the >> latter, then why replace the SPS in-band but never the PPS? >> The in-band SPS is virtually unused after this change. > > My guess would be that someone (or some/thing/, dun dun dun) did some > botched/misunderstood processing on that sample, so trying to figure out > what was intended is futile. > > According to the spec, SPS contents must not change within a sequence > (they can change on sequence boundaries), so this sample is invalid. While this sample is invalid, isn't it possible to have a sample as follows: First a SPS and a PPS referencing the SPS, then a coded video sequence for which these SPS and PPS are active, then another IDR access unit with a new SPS that overwrites the old one, but with no new PPS. This is valid if I am not mistaken. With your patch, the second SPS would never be used. - Andreas ___ 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] movenc: mark Opus encapsulation as stable
The specifications are de-facto frozen now as they've already been used in production for years now, the author has indicated reluctance on IRC to change it further, and the only potential changes would, from what I understand, be forward-compatible. Patch attached. >From c8d4d5a3d7aeaddabb9e14bcf5816f3b1a7a3290 Mon Sep 17 00:00:00 2001 From: Lynne Date: Mon, 23 Mar 2020 22:03:24 + Subject: [PATCH] movenc: mark Opus encapsulation as stable The specifications are de-facto frozen now as they've already been used in production for years now, the author has indicated reluctance on IRC to change it further, and the only potential changes would, from what I understand, be forward-compatible. --- libavformat/movenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 85df5d1374..4e56a7df46 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -6500,7 +6500,8 @@ static int mov_init(AVFormatContext *s) av_log(s, AV_LOG_ERROR, "%s only supported in MP4.\n", avcodec_get_name(track->par->codec_id)); return AVERROR(EINVAL); } -if (s->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) { +if (track->par->codec_id != AV_CODEC_ID_OPUS && +s->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) { av_log(s, AV_LOG_ERROR, "%s in MP4 support is experimental, add " "'-strict %d' if you want to use it.\n", -- 2.26.0.rc2 ___ 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] movenc: mark Opus encapsulation as stable
On 23/03/2020 22:11, Lynne wrote: > The specifications are de-facto frozen now as they've already been used in > production for years now, the author has indicated reluctance on IRC to change > it further, and the only potential changes would, from what I understand, be > forward-compatible. +1 from me for all the same reasons. I am actually surprised it was still marked as experimental. - 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/wavpack: Prevent frame format from being wrong
On 3/23/20 9:49 AM, Anton Khirnov wrote: > Quoting Michael Niedermayer (2020-03-20 21:50:18) >> On Fri, Mar 20, 2020 at 10:18:49AM +0100, Anton Khirnov wrote: >>> Quoting Michael Niedermayer (2020-03-20 01:03:36) Fixes: out of array access Fixes: 21193/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVPACK_fuzzer-5125168956702720 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/wavpack.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c index b27262b94e..e9c870e41e 100644 --- a/libavcodec/wavpack.c +++ b/libavcodec/wavpack.c @@ -1488,6 +1488,7 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no, /* get output buffer */ wc->curr_frame.f->nb_samples = s->samples; +wc->curr_frame.f->format = avctx->sample_fmt; >>> How does this have any effect? curr_frame.f should now be clean and get >>> initialized from avctx->sample_fmt. >> IIRC >> The format changes between frames, so the struct is still set to the one >> from the previous frame and that overrides the use of the avctx value >> >> setting it to NONE (here or somewhere else) should work too. > ff_thread_release_buffer() is called on that frame immediately before, > which should reset it to defaults (setting format to FMT_NONE). > I don't think the format should change between frames, so I don't understand how the format is getting set to a wacky value. Would it be possible for me the get the triggering test case and try this myself? I searched and couldn't find it, so I assume it's not public yet. I assume that just decoding the file should trigger the assertion, right? 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 2/2] avcodec/wavpack: Prevent frame format from being wrong
On Mon, Mar 23, 2020 at 05:08:23PM -0700, David Bryant wrote: > On 3/23/20 9:49 AM, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2020-03-20 21:50:18) > >> On Fri, Mar 20, 2020 at 10:18:49AM +0100, Anton Khirnov wrote: > >>> Quoting Michael Niedermayer (2020-03-20 01:03:36) > Fixes: out of array access > Fixes: > 21193/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVPACK_fuzzer-5125168956702720 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/wavpack.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c > index b27262b94e..e9c870e41e 100644 > --- a/libavcodec/wavpack.c > +++ b/libavcodec/wavpack.c > @@ -1488,6 +1488,7 @@ static int wavpack_decode_block(AVCodecContext > *avctx, int block_no, > > /* get output buffer */ > wc->curr_frame.f->nb_samples = s->samples; > +wc->curr_frame.f->format = avctx->sample_fmt; > >>> How does this have any effect? curr_frame.f should now be clean and get > >>> initialized from avctx->sample_fmt. > >> IIRC > >> The format changes between frames, so the struct is still set to the one > >> from the previous frame and that overrides the use of the avctx value > >> > >> setting it to NONE (here or somewhere else) should work too. > > ff_thread_release_buffer() is called on that frame immediately before, > > which should reset it to defaults (setting format to FMT_NONE). > > > I don't think the format should change between frames, so I don't understand > how the format is getting set to a wacky value. wavpack_decode_frame() sets the format from flags read for each frame. > > Would it be possible for me the get the triggering test case and try this > myself? I searched and couldn't find it, so I assume > it's not public yet. I assume that just decoding the file should trigger the > assertion, right? sample sent privatly thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 1 "Used only once"- "Some unspecified defect prevented a second use" "In good condition" - "Can be repaird by experienced expert" "As is" - "You wouldnt want it even if you were payed for it, if you knew ..." signature.asc Description: PGP signature ___ 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] avcodec/wavpack: Prevent frame format from being wrong
On Mon, Mar 23, 2020 at 05:49:06PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2020-03-20 21:50:18) > > On Fri, Mar 20, 2020 at 10:18:49AM +0100, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2020-03-20 01:03:36) > > > > Fixes: out of array access > > > > Fixes: > > > > 21193/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVPACK_fuzzer-5125168956702720 > > > > > > > > Found-by: continuous fuzzing process > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer > > > > --- > > > > libavcodec/wavpack.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c > > > > index b27262b94e..e9c870e41e 100644 > > > > --- a/libavcodec/wavpack.c > > > > +++ b/libavcodec/wavpack.c > > > > @@ -1488,6 +1488,7 @@ static int wavpack_decode_block(AVCodecContext > > > > *avctx, int block_no, > > > > > > > > /* get output buffer */ > > > > wc->curr_frame.f->nb_samples = s->samples; > > > > +wc->curr_frame.f->format = avctx->sample_fmt; > > > > > > How does this have any effect? curr_frame.f should now be clean and get > > > initialized from avctx->sample_fmt. > > > > IIRC > > The format changes between frames, so the struct is still set to the one > > from the previous frame and that overrides the use of the avctx value > > > > setting it to NONE (here or somewhere else) should work too. > > ff_thread_release_buffer() is called on that frame immediately before, > which should reset it to defaults (setting format to FMT_NONE). ff_thread_release_buffer() does not reset it in all cases Ill send an alternative patch, which does the reset in the get side on failure. We already have some reset code for dimensions there. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Into a blind darkness they enter who follow after the Ignorance, they as if into a greater darkness enter who devote themselves to the Knowledge alone. -- Isha Upanishad signature.asc Description: PGP signature ___ 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] avcodec/decode: Clear format on ff_get_buffer() failure
Fixes: out of array access Fixes: 21193/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVPACK_fuzzer-5125168956702720 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/decode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index af6bb3f952..fb22ef0ef3 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1972,6 +1972,7 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags) if (ret < 0) { av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n"); frame->width = frame->height = 0; +frame->format = -1; } return ret; } -- 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".
[FFmpeg-devel] [PATCH] Check return value from avio_read() to verify data actually read
Chromium fuzzers have caught places where uninitialized data was used due to calls to avio_read() not verifying that the number of bytes expected was actually read. So updating the code to check the result from avio_read(). 0001-Check-return-value-from-avio_read-to-verify-data-act.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 3/3] lavc/vaapi_encode_h265: add h265 tile encoding support
> From: myp...@gmail.com > Sent: Monday, March 23, 2020 22:44 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Cc: Fu, Linjie > Subject: Re: [FFmpeg-devel] [PATCH 3/3] lavc/vaapi_encode_h265: add h265 > tile encoding support > > On Mon, Mar 23, 2020 at 10:30 PM Linjie Fu wrote: > > > > Default to enable uniform_spacing_flag. Guess level by the tile > > rows/cols. Supported for ICL+ platforms. > > > > Also add documentations. > > > > To encode with 4 rows 2 columns: > > ffmpeg ... -c:v hevc_vaapi -tile_rows 4 -tile_cols 2 ... > > > > Signed-off-by: Linjie Fu > > --- > > doc/encoders.texi | 8 > > libavcodec/vaapi_encode_h265.c | 36 > +++- > > 2 files changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/doc/encoders.texi b/doc/encoders.texi > > index e23b6b3..2dc5cd4 100644 > > --- a/doc/encoders.texi > > +++ b/doc/encoders.texi > > @@ -3091,6 +3091,14 @@ Include HDR metadata if the input frames have it > > messages). > > @end table > > > > +@item tile_rows > > +Selects how many rows of tiles to encode with. For example, 4 tile rows > would > > +be requested by setting the tile_rows option to 4. > > + > > +@item tile_cols > > +Selects how many columns of tiles to encode with. For example, 5 tile > columns > > +would be requested by setting the tile_cols option to 5. > > + > > @end table > > > > @item mjpeg_vaapi > > diff --git a/libavcodec/vaapi_encode_h265.c > b/libavcodec/vaapi_encode_h265.c > > index 97dc5a7..457e3d9 100644 > > --- a/libavcodec/vaapi_encode_h265.c > > +++ b/libavcodec/vaapi_encode_h265.c > > @@ -63,6 +63,9 @@ typedef struct VAAPIEncodeH265Context { > > int level; > > int sei; > > > > +int trows; > > +int tcols; > > + > > // Derived settings. > > int fixed_qp_idr; > > int fixed_qp_p; > > @@ -345,7 +348,7 @@ static int > vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) > > > > level = ff_h265_guess_level(ptl, avctx->bit_rate, > > ctx->surface_width, > > ctx->surface_height, > > -ctx->nb_slices, 1, 1, > > +ctx->nb_slices, ctx->tile_rows, > > ctx->tile_cols, > > (ctx->b_per_p > 0) + 1); > > if (level) { > > av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", > > level->name); > > @@ -558,6 +561,20 @@ static int > vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) > > > > pps->pps_loop_filter_across_slices_enabled_flag = 1; > > > > +if (ctx->tile_rows && ctx->tile_cols) { > > +pps->tiles_enabled_flag = 1; > > +pps->uniform_spacing_flag = 1; > > + > > +pps->num_tile_rows_minus1 = ctx->tile_rows - 1; > > +pps->num_tile_columns_minus1= ctx->tile_cols - 1; > > + > > +pps->loop_filter_across_tiles_enabled_flag = 1; > > + > > +for (i = 0; i <= pps->num_tile_rows_minus1; i++) > > +pps->row_height_minus1[i] = ctx->row_height[i] - 1; > > +for (i = 0; i <= pps->num_tile_columns_minus1; i++) > > +pps->column_width_minus1[i] = ctx->col_width[i] - 1; > > +} > > > > // Fill VAAPI parameter buffers. > > > > @@ -666,6 +683,13 @@ static int > vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx) > > }, > > }; > > > > +if (pps->tiles_enabled_flag) { > > +for (i = 0; i <= vpic->num_tile_rows_minus1; i++) > > +vpic->row_height_minus1[i] = pps->row_height_minus1[i]; > > +for (i = 0; i <= vpic->num_tile_columns_minus1; i++) > > +vpic->column_width_minus1[i] = pps->column_width_minus1[i]; > > +} > > + > > return 0; > > } > > > > @@ -1181,6 +1205,11 @@ static av_cold int > vaapi_encode_h265_init(AVCodecContext *avctx) > > if (priv->qp > 0) > > ctx->explicit_qp = priv->qp; > > > > +if (priv->trows && priv->tcols) { > > +ctx->tile_rows = priv->trows; > > +ctx->tile_cols = priv->tcols; > > +} > > + > > return ff_vaapi_encode_init(avctx); > > } > > > > @@ -1257,6 +1286,11 @@ static const AVOption > vaapi_encode_h265_options[] = { > >{ .i64 = SEI_MASTERING_DISPLAY | SEI_CONTENT_LIGHT_LEVEL }, > >INT_MIN, INT_MAX, FLAGS, "sei" }, > > > > +{ "tile_rows", "Number of rows for tile encoding", > > + OFFSET(trows), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS }, > > +{ "tile_cols", "Number of cols for tile encoding", > > + OFFSET(tcols), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS }, > > + > > { NULL }, > > }; > > > > -- > > As my viewpoint, -tiles 3x2 (rows x cols) more naturally Yep, saw identical options in libaomenc[1] which support "-tiles", "- tile-columns", "- tile-rows" . Add -tiles option as well for more natural usages: +{ "tiles", "Tile rows x cols", + OFFSET(trows), AV_OPT_TYPE_IMAGE_
Re: [FFmpeg-devel] [PATCH 04/21] avformat: Redo cleanup of demuxer upon read_header() failure
Andreas Rheinhardt: > James Almer: >> On 3/22/2020 12:47 AM, Andreas Rheinhardt wrote: >>> If reading the header fails, the demuxer's read_close() function (if >>> existing) is not called automatically; instead several demuxers call it >>> via "goto fail" in read_header(). >>> >>> This commit intends to change this by adding a flag to AVInputFormat >>> that can be used to set on a per-AVInputFormat basis whether read_close() >>> should be called generically after an error during read_header(). >>> >>> The flag controlling this behaviour has been added because it might be >>> unsafe to call read_close() generally (e.g. this might lead to >>> read_close() being called twice and this might e.g. lead to double-frees >>> if av_free() instead of av_freep() is used; or a size field has not >>> been reset after freeing the elements (see the mov demuxer for an >>> example of this)). Yet the intention is to check and fix all demuxers >>> and make the flag redundant in the medium run. >>> >>> The flag has been added to a new internal field of AVInputFormat, >>> flags_internal. When it has become redundant, it can be removed again >>> at the next major version bump. >> >> Fields considered not public (And thus added below the relevant notice >> in the struct) can be added and removed at any time. So this sentence is >> not needed. >> > This field is not public, but I thought that the fact that libavdevice > also defines AVInputFormats makes this de-facto avpriv. Isn't it allowed > to use a newer libavdevice together with an older libavformat? Imagine > your libavdevice to be so new that the field has already been removed, > yet the libavformat being so old that it still checks for the field. > That would not go well. > (If I am right then this gives the task of checking and adapting all the > demuxers a certain urgency to be finished before the next major bump.) > I reconsidered this and I think I got it exactly wrong: libavdevice is linked against libavformat, not the other way around. Therefore one can swap the libavformat against a newer one, but one is not allowed to update libavdevice without also updating libavformat. (This presumes that the usual rules apply in this scenario; I haven't seen any exception documented for the special case of the heavily interwoven libavdevice and libavformat, so I guess there is none.) This means that adding new fields to AVInputFormat (and using them) is not possible, because said AVInputFormat might come from an older version of libavdevice and might therefore not have these fields. This means that my proposal has to be modified. flags_internal is not feasible (unless one uses a version check for. One could use a not publically documented flag of the normal flags. This also means that this flag can not be enabled by default (and therefore removed) before the next major version bump. E.g. the alsa input device seems to be incompatible with automatically running its read_close function after failure of reading the header (the current error handling closes a snd_pcm_t, but doesn't reset the pointer to NULL). - Andreas ___ 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] JPEG2000 Decoder/Encoder Enhancements
Hi Michael, I do not mind spending most of the time myself. I just need a person to go to when I have doubts. I really do not mind doing it without it being through GSoC either. Just wanted to try contributing to the organization. I was thinking about starting by fixing tickets #4601 or #4610. Would that be a good way to start? On Mon, Mar 23, 2020 at 10:33 PM Michael Niedermayer wrote: > > On Sun, Mar 22, 2020 at 06:58:24PM +0100, Carl Eugen Hoyos wrote: > > Am So., 22. März 2020 um 18:27 Uhr schrieb Gautam Ramakrishnan > > : > > > > > I saw the list of unmentored projects for GSoC and enhancements to the > > > JPEG2000 decoder and encoder were there. I am finding it very > > > difficult to make a proposal as the specifications are not available > > > for free. However, I am willing to work on improving the JPEG2000 > > > features even if it is not through GSoC if someone can guide me > > > through it. > > > > > > I will basically need some guidance on the specifications and some > > > advice on how to go about implementing the missing features. Could I > > > take this up even if it is not a part of GSoC? > > > > Google can find an (old but probably usable) version of the standard. > > > > You can only work on this task if you find a mentor;-( > > i can help mentoring a jpeg2000 related project but the standard is a bit > convoluted. And while i do know jpeg2000 a bit iam no jpeg2000 expert. > So for many detail questions i would have to spend considerable time > looking through the specification. I dont have the time for doing that > alot > So if Gautam can wrap his head around the specification and has the time > to decipher the details where needed. Meaning if the majority of the > time spend is on Gautam or whoever does the work then i can (help) mentor > it probably > > Thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I know you won't believe me, but the highest form of Human Excellence is > to question oneself and others. -- Socrates > ___ > 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". -- - Gautam | ___ 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 v3] avformat/dashdec: fix memleak for commit commit e134c203
These member will be used for get more correct information of the MPD Suggested-by: Andreas Rheinhardt Suggested-by: Nicolas George Signed-off-by: Steven Liu --- libavformat/dashdec.c | 244 -- 1 file changed, 214 insertions(+), 30 deletions(-) diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c index 5bbe5d3985..b5bb8e674c 100644 --- a/libavformat/dashdec.c +++ b/libavformat/dashdec.c @@ -108,6 +108,7 @@ struct representation { int64_t cur_seg_offset; int64_t cur_seg_size; struct fragment *cur_seg; +char *lang; /* Currently active Media Initialization Section */ struct fragment *init_section; @@ -122,19 +123,15 @@ struct representation { typedef struct DASHContext { const AVClass *class; char *base_url; -char *adaptionset_contenttype_val; -char *adaptionset_par_val; -char *adaptionset_lang_val; -char *adaptionset_minbw_val; -char *adaptionset_maxbw_val; -char *adaptionset_minwidth_val; -char *adaptionset_maxwidth_val; -char *adaptionset_minheight_val; -char *adaptionset_maxheight_val; -char *adaptionset_minframerate_val; -char *adaptionset_maxframerate_val; -char *adaptionset_segmentalignment_val; -char *adaptionset_bitstreamswitching_val; +char *adaptionset_lang; +uint64_t adaptionset_minbw; +uint64_t adaptionset_maxbw; +uint64_t adaptionset_minwidth; +uint64_t adaptionset_maxwidth; +uint64_t adaptionset_minheight; +uint64_t adaptionset_maxheight; +AVRational adaptionset_minframerate; +AVRational adaptionset_maxframerate; int n_videos; struct representation **videos; @@ -169,6 +166,79 @@ typedef struct DASHContext { } DASHContext; +static int get_ratio_from_string(AVRational *dst, char *src) +{ +char *p = src; +char *end = NULL; +char *end_ptr = NULL; +int num, den; + +num = (int)strtol(p, &end_ptr, 10); +if (errno == ERANGE || end_ptr == src) { +return AVERROR_INVALIDDATA; +} + +if (end_ptr[0] == '\0') { +dst->den = 1; +dst->num = num; +return 0; +} + +if (end_ptr[0] != '/') +return AVERROR_INVALIDDATA; +p = end_ptr + 1; +den = (int)strtol(p, &end, 10); +if (errno == ERANGE || end == src) { +dst->den = 1; +return AVERROR_INVALIDDATA; +} +dst->den = den; + +return 0; +} + +static uint64_t dash_prop_get_int64(AVFormatContext *s, xmlNodePtr node, uint64_t *dst, const char *key) +{ +char *end_ptr = NULL; +char *val = xmlGetProp(node, key); +int ret = 0; + +if (val) { +ret = strtoull(val, &end_ptr, 10); +if (errno == ERANGE) { +av_log(s, AV_LOG_WARNING, "overflow/underflow when get value of %s\n", key); +xmlFree(val); +return AVERROR_INVALIDDATA; +} +if (end_ptr == val || end_ptr[0] != '\0') { +av_log(s, AV_LOG_ERROR, "The %s field value is " +"not a valid number: %s\n", key, val); +xmlFree(val); +return AVERROR_INVALIDDATA; +} +*dst = ret; +xmlFree(val); +} +return ret; +} + +static int dash_get_prop_ratio(AVFormatContext *s, xmlNodePtr node, AVRational *member, const char *key) +{ +char *val = xmlGetProp(node, key); +int ret = 0; +AVRational rate; + +if (val) { +ret = get_ratio_from_string(&rate, val); +if (ret < 0) +av_log(s, AV_LOG_WARNING, "Ignoring invalid %s frame rate '%s'\n", key, val); +xmlFree(val); +} +member->num = rate.num; +member->den = rate.den; +return ret; +} + static int ishttp(char *url) { const char *proto_name = avio_find_protocol_name(url); @@ -885,6 +955,15 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url, ret = AVERROR(ENOMEM); goto end; } +if (c->adaptionset_lang) { +rep->lang = av_strdup(c->adaptionset_lang); +if (!rep->lang) { +av_log(s, AV_LOG_ERROR, "alloc language memory failure\n"); +av_freep(&rep); +ret = AVERROR(ENOMEM); +goto end; +} +} rep->parent = s; representation_segmenttemplate_node = find_child_node_by_name(representation_node, "SegmentTemplate"); representation_baseurl_node = find_child_node_by_name(representation_node, "BaseURL"); @@ -1070,15 +1149,61 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url, } if (rep) { +uint64_t width = 0; +uint64_t height = 0; + if (rep->fragment_duration > 0 && !rep->fragment_timescale) rep->fragment_timescale = 1; rep->bandwidth = rep_bandwidth_val ? atoi(rep_bandwidth_val) : 0; +if (rep_bandwidth_val && (rep->bandwidth > c->adaptionset_maxbw ||
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/motion_est: fix penalty_factor for b frames
On Mon, Mar 23, 2020 at 10:42 PM Michael Niedermayer wrote: > On Sun, Mar 22, 2020 at 04:55:25PM +0100, Ramiro Polla wrote: > > In ff_estimate_b_frame_motion(), penalty_factor would be used before > > being initialized in estimate_motion_b(). Also, the initialization > > would happen more than once unnecessarily. > > --- > > libavcodec/motion_est.c | 15 --- > > tests/ref/vsynth/vsynth2-mpeg2-422 | 6 +++--- > > tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd | 6 +++--- > > tests/ref/vsynth/vsynth2-mpeg4-adap | 6 +++--- > > 4 files changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c > > index 02c75fd470..1feb46cec3 100644 > > --- a/libavcodec/motion_est.c > > +++ b/libavcodec/motion_est.c > > @@ -1123,9 +1123,6 @@ static int estimate_motion_b(MpegEncContext *s, int > > mb_x, int mb_y, > > uint8_t * const mv_penalty= c->mv_penalty[f_code] + MAX_DMV; > > int mv_scale; > > > > -c->penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > > c->avctx->me_cmp); > > -c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > > c->avctx->me_sub_cmp); > > -c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, > > c->avctx->mb_cmp); > > c->current_mv_penalty= mv_penalty; > > > > get_limits(s, 16*mb_x, 16*mb_y); > > > > > @@ -1491,7 +1488,6 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > > int mb_x, int mb_y) > > { > > MotionEstContext * const c= &s->me; > > -const int penalty_factor= c->mb_penalty_factor; > > int fmin, bmin, dmin, fbmin, bimin, fimin; > > int type=0; > > const int xy = mb_y*s->mb_stride + mb_x; > > @@ -1517,18 +1513,23 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > > dmin= direct_search(s, mb_x, mb_y); > > else > > dmin= INT_MAX; > > + > > +c->penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > > c->avctx->me_cmp); > > +c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, > > c->avctx->me_sub_cmp); > > +c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, > > c->avctx->mb_cmp); > > If mb_penalty_factor isnt correct in this before this maybe isnt enough > as the direct_search() uses mb_penalty_factor Fixed. New patch attached. From 8feded1143715b064c8556a460feb86394b86acd Mon Sep 17 00:00:00 2001 From: Ramiro Polla Date: Sun, 22 Mar 2020 16:45:05 +0100 Subject: [PATCH] avcodec/motion_est: fix penalty_factor for b frames In direct_search() and ff_estimate_b_frame_motion(), penalty_factor would be used before being initialized in estimate_motion_b(). Also, the initialization would happen more than once unnecessarily. --- libavcodec/motion_est.c | 15 --- tests/ref/vsynth/vsynth1-mpeg4-thread| 6 +++--- tests/ref/vsynth/vsynth2-mpeg2-422 | 6 +++--- tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd | 6 +++--- tests/ref/vsynth/vsynth2-mpeg4-adap | 6 +++--- tests/ref/vsynth/vsynth2-mpeg4-qprd | 6 +++--- tests/ref/vsynth/vsynth2-mpeg4-thread| 6 +++--- tests/ref/vsynth/vsynth_lena-mpeg4-rc| 4 ++-- 8 files changed, 28 insertions(+), 27 deletions(-) diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c index 02c75fd470..520a57d4d9 100644 --- a/libavcodec/motion_est.c +++ b/libavcodec/motion_est.c @@ -1123,9 +1123,6 @@ static int estimate_motion_b(MpegEncContext *s, int mb_x, int mb_y, uint8_t * const mv_penalty= c->mv_penalty[f_code] + MAX_DMV; int mv_scale; -c->penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); -c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); -c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); c->current_mv_penalty= mv_penalty; get_limits(s, 16*mb_x, 16*mb_y); @@ -1491,7 +1488,6 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, int mb_x, int mb_y) { MotionEstContext * const c= &s->me; -const int penalty_factor= c->mb_penalty_factor; int fmin, bmin, dmin, fbmin, bimin, fimin; int type=0; const int xy = mb_y*s->mb_stride + mb_x; @@ -1513,22 +1509,27 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, return; } +c->penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); +c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); +c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); + if (s->codec_id == AV_CODEC_ID_MPEG4) dmin= direct_search(s, mb_x, mb_y); else dmin= INT_MAX; + // FIXME penalty stuff for non-MPEG-4 c->skip=0; fmin = estimate_motion_b(s, mb_x, mb_y, s->b_forw_mv_table, 0, s->f_code) + - 3 * penalty_factor; + 3 * c->mb_penalty_factor;