| Message ID | 20190903001426.8957-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
NAK, you can use max_pixels for the same cause. On 9/3/19, Michael Niedermayer <michael@niedermayer.cc> wrote: > TODO: APIChanges, bump version > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/avcodec.h | 8 ++++++++ > libavcodec/options_table.h | 1 + > tests/ref/fate/api-mjpeg-codec-param | 2 ++ > tests/ref/fate/api-png-codec-param | 2 ++ > 4 files changed, 13 insertions(+) > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index de4feb6a65..4b771a6ae6 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -3371,6 +3371,14 @@ typedef struct AVCodecContext { > * - encoding: unused > */ > int discard_damaged_percentage; > + > + /** > + * The number of samples per frame to maximally accept. > + * > + * - decoding: set by user > + * - encoding: set by user > + */ > + int64_t max_samples; > } AVCodecContext; > > #if FF_API_CODEC_GET_SET > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h > index c3a500a629..b56e653bf6 100644 > --- a/libavcodec/options_table.h > +++ b/libavcodec/options_table.h > @@ -476,6 +476,7 @@ static const AVOption avcodec_options[] = { > {"pixel_format", "set pixel format", OFFSET(pix_fmt), > AV_OPT_TYPE_PIXEL_FMT, {.i64=AV_PIX_FMT_NONE}, -1, INT_MAX, 0 }, > {"video_size", "set video size", OFFSET(width), AV_OPT_TYPE_IMAGE_SIZE, > {.str=NULL}, 0, INT_MAX, 0 }, > {"max_pixels", "Maximum number of pixels", OFFSET(max_pixels), > AV_OPT_TYPE_INT64, {.i64 = INT_MAX }, 0, INT_MAX, A|V|S|D|E }, > +{"max_samples", "Maximum number of samples", OFFSET(max_samples), > AV_OPT_TYPE_INT64, {.i64 = INT_MAX }, 0, INT_MAX, A|D|E }, > {"hwaccel_flags", NULL, OFFSET(hwaccel_flags), AV_OPT_TYPE_FLAGS, {.i64 = > AV_HWACCEL_FLAG_IGNORE_LEVEL }, 0, UINT_MAX, V|D, "hwaccel_flags"}, > {"ignore_level", "ignore level even if the codec level used is unknown or > higher than the maximum supported level reported by the hardware driver", 0, > AV_OPT_TYPE_CONST, { .i64 = AV_HWACCEL_FLAG_IGNORE_LEVEL }, INT_MIN, > INT_MAX, V | D, "hwaccel_flags" }, > {"allow_high_depth", "allow to output YUV pixel formats with a different > chroma sampling than 4:2:0 and/or other than 8 bits per component", 0, > AV_OPT_TYPE_CONST, {.i64 = AV_HWACCEL_FLAG_ALLOW_HIGH_DEPTH }, INT_MIN, > INT_MAX, V | D, "hwaccel_flags"}, > diff --git a/tests/ref/fate/api-mjpeg-codec-param > b/tests/ref/fate/api-mjpeg-codec-param > index 0815919d7d..e55cef0eb9 100644 > --- a/tests/ref/fate/api-mjpeg-codec-param > +++ b/tests/ref/fate/api-mjpeg-codec-param > @@ -136,6 +136,7 @@ stream=0, decode=0 > pixel_format=yuvj422p > video_size=400x225 > max_pixels=2147483647 > + max_samples=2147483647 > hwaccel_flags=0x00000001 > extra_hw_frames=-1 > discard_damaged_percentage=95 > @@ -277,6 +278,7 @@ stream=0, decode=1 > pixel_format=yuvj422p > video_size=400x225 > max_pixels=2147483647 > + max_samples=2147483647 > hwaccel_flags=0x00000001 > extra_hw_frames=-1 > discard_damaged_percentage=95 > diff --git a/tests/ref/fate/api-png-codec-param > b/tests/ref/fate/api-png-codec-param > index a47d0963da..c04c8cc7c1 100644 > --- a/tests/ref/fate/api-png-codec-param > +++ b/tests/ref/fate/api-png-codec-param > @@ -136,6 +136,7 @@ stream=0, decode=0 > pixel_format=rgba > video_size=128x128 > max_pixels=2147483647 > + max_samples=2147483647 > hwaccel_flags=0x00000001 > extra_hw_frames=-1 > discard_damaged_percentage=95 > @@ -277,6 +278,7 @@ stream=0, decode=1 > pixel_format=rgba > video_size=128x128 > max_pixels=2147483647 > + max_samples=2147483647 > hwaccel_flags=0x00000001 > extra_hw_frames=-1 > discard_damaged_percentage=95 > -- > 2.23.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-request@ffmpeg.org with subject "unsubscribe".
tis 2019-09-03 klockan 09:20 +0200 skrev Paul B Mahol: > > On 9/3/19, Michael Niedermayer <michael@niedermayer.cc> wrote: > > TODO: APIChanges, bump version > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/avcodec.h | 8 ++++++++ > > libavcodec/options_table.h | 1 + > > tests/ref/fate/api-mjpeg-codec-param | 2 ++ > > tests/ref/fate/api-png-codec-param | 2 ++ > > 4 files changed, 13 insertions(+) > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > > index de4feb6a65..4b771a6ae6 100644 > > --- a/libavcodec/avcodec.h > > +++ b/libavcodec/avcodec.h > > @@ -3371,6 +3371,14 @@ typedef struct AVCodecContext { > > * - encoding: unused > > */ > > int discard_damaged_percentage; > > + > > + /** > > + * The number of samples per frame to maximally accept. > > + * > > + * - decoding: set by user > > + * - encoding: set by user > > + */ > > + int64_t max_samples; > > } AVCodecContext; > > NAK, you can use max_pixels for the same cause. Or the other way around. Has max_pixels made it to an official release yet? Using it for samples is obviously confusing Hot take: AVCodec is way overdue for a split into ACodec and VCodec (and SCodec and so on), with AVCodec being a typed union /Tomas
On Tue, Sep 03, 2019 at 09:20:52AM +0200, Paul B Mahol wrote:
> NAK, you can use max_pixels for the same cause.
yes, we can, its poor design though audio samples are not
pixels.
Anyone else has an opinion on this ?
Thanks
[...]
On Tue, Sep 03, 2019 at 12:21:26PM +0200, Tomas Härdin wrote: > tis 2019-09-03 klockan 09:20 +0200 skrev Paul B Mahol: > > > > On 9/3/19, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > TODO: APIChanges, bump version > > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavcodec/avcodec.h | 8 ++++++++ > > > libavcodec/options_table.h | 1 + > > > tests/ref/fate/api-mjpeg-codec-param | 2 ++ > > > tests/ref/fate/api-png-codec-param | 2 ++ > > > 4 files changed, 13 insertions(+) > > > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > > > index de4feb6a65..4b771a6ae6 100644 > > > --- a/libavcodec/avcodec.h > > > +++ b/libavcodec/avcodec.h > > > @@ -3371,6 +3371,14 @@ typedef struct AVCodecContext { > > > * - encoding: unused > > > */ > > > int discard_damaged_percentage; > > > + > > > + /** > > > + * The number of samples per frame to maximally accept. > > > + * > > > + * - decoding: set by user > > > + * - encoding: set by user > > > + */ > > > + int64_t max_samples; > > > } AVCodecContext; > > > > NAK, you can use max_pixels for the same cause. > > Or the other way around. Has max_pixels made it to an official release > yet? max_pixels is in AVCodecContext since 3.3 but even if it wasnt. while no question max_samples would be a much better name for a unified field than max_pixels. We do not unify other related fields like width & height with for example nb_samples and channels There is also a semantic difference between max_pixels and max_samples the max_pixels are the samples at one instance of time while max_samples cover a period of time depending on the duration of a packet if ever a video codec had packets covering a period of time the same way audio does (like maybe 0.5 seconds or 15 frames) we would not apply max_pixels to the whole but to the 15 frames individually So IMHO its possible to use the same field for both but it feels a bit like bending 2 slightly different things onto each other. But sure if people prefer it ill change the patches. > Using it for samples is obviously confusing yes thanks [...]
tis 2019-09-03 klockan 13:10 +0200 skrev Michael Niedermayer: > On Tue, Sep 03, 2019 at 12:21:26PM +0200, Tomas Härdin wrote: > > tis 2019-09-03 klockan 09:20 +0200 skrev Paul B Mahol: > > > On 9/3/19, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > TODO: APIChanges, bump version > > > > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavcodec/avcodec.h | 8 ++++++++ > > > > libavcodec/options_table.h | 1 + > > > > tests/ref/fate/api-mjpeg-codec-param | 2 ++ > > > > tests/ref/fate/api-png-codec-param | 2 ++ > > > > 4 files changed, 13 insertions(+) > > > > > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > > > > index de4feb6a65..4b771a6ae6 100644 > > > > --- a/libavcodec/avcodec.h > > > > +++ b/libavcodec/avcodec.h > > > > @@ -3371,6 +3371,14 @@ typedef struct AVCodecContext { > > > > * - encoding: unused > > > > */ > > > > int discard_damaged_percentage; > > > > + > > > > + /** > > > > + * The number of samples per frame to maximally accept. > > > > + * > > > > + * - decoding: set by user > > > > + * - encoding: set by user > > > > + */ > > > > + int64_t max_samples; > > > > } AVCodecContext; > > > > > > NAK, you can use max_pixels for the same cause. > > > > Or the other way around. Has max_pixels made it to an official release > > yet? > > max_pixels is in AVCodecContext since 3.3 > > but even if it wasnt. while no question max_samples would be a much > better name for a unified field than max_pixels. > > We do not unify other related fields like > width & height with for example nb_samples and channels > > There is also a semantic difference between max_pixels and max_samples > > the max_pixels are the samples at one instance of time > while max_samples cover a period of time depending on the duration of a packet This is a good point. max_pixels as applied to audio would then mean maximum number of channels. I still think putting audio and video stuff in the same struct is a bit of a mistake, for just this reason /Tomas
On 9/3/2019 7:57 AM, Michael Niedermayer wrote: > On Tue, Sep 03, 2019 at 09:20:52AM +0200, Paul B Mahol wrote: >> NAK, you can use max_pixels for the same cause. > > yes, we can, its poor design though audio samples are not > pixels. > > Anyone else has an opinion on this ? > > Thanks I prefer a separate field. If the existing field was called in a more ambiguous way I'd be ok with reusing it, but not like this.
James Almer (12019-09-03): > I prefer a separate field. If the existing field was called in a more > ambiguous way I'd be ok with reusing it, but not like this. Same for me. Regards,
On Tue, Sep 03, 2019 at 07:52:35PM +0200, Nicolas George wrote: > James Almer (12019-09-03): > > I prefer a separate field. If the existing field was called in a more > > ambiguous way I'd be ok with reusing it, but not like this. > > Same for me. ok, will apply thanks [...]
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index de4feb6a65..4b771a6ae6 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -3371,6 +3371,14 @@ typedef struct AVCodecContext { * - encoding: unused */ int discard_damaged_percentage; + + /** + * The number of samples per frame to maximally accept. + * + * - decoding: set by user + * - encoding: set by user + */ + int64_t max_samples; } AVCodecContext; #if FF_API_CODEC_GET_SET diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h index c3a500a629..b56e653bf6 100644 --- a/libavcodec/options_table.h +++ b/libavcodec/options_table.h @@ -476,6 +476,7 @@ static const AVOption avcodec_options[] = { {"pixel_format", "set pixel format", OFFSET(pix_fmt), AV_OPT_TYPE_PIXEL_FMT, {.i64=AV_PIX_FMT_NONE}, -1, INT_MAX, 0 }, {"video_size", "set video size", OFFSET(width), AV_OPT_TYPE_IMAGE_SIZE, {.str=NULL}, 0, INT_MAX, 0 }, {"max_pixels", "Maximum number of pixels", OFFSET(max_pixels), AV_OPT_TYPE_INT64, {.i64 = INT_MAX }, 0, INT_MAX, A|V|S|D|E }, +{"max_samples", "Maximum number of samples", OFFSET(max_samples), AV_OPT_TYPE_INT64, {.i64 = INT_MAX }, 0, INT_MAX, A|D|E }, {"hwaccel_flags", NULL, OFFSET(hwaccel_flags), AV_OPT_TYPE_FLAGS, {.i64 = AV_HWACCEL_FLAG_IGNORE_LEVEL }, 0, UINT_MAX, V|D, "hwaccel_flags"}, {"ignore_level", "ignore level even if the codec level used is unknown or higher than the maximum supported level reported by the hardware driver", 0, AV_OPT_TYPE_CONST, { .i64 = AV_HWACCEL_FLAG_IGNORE_LEVEL }, INT_MIN, INT_MAX, V | D, "hwaccel_flags" }, {"allow_high_depth", "allow to output YUV pixel formats with a different chroma sampling than 4:2:0 and/or other than 8 bits per component", 0, AV_OPT_TYPE_CONST, {.i64 = AV_HWACCEL_FLAG_ALLOW_HIGH_DEPTH }, INT_MIN, INT_MAX, V | D, "hwaccel_flags"}, diff --git a/tests/ref/fate/api-mjpeg-codec-param b/tests/ref/fate/api-mjpeg-codec-param index 0815919d7d..e55cef0eb9 100644 --- a/tests/ref/fate/api-mjpeg-codec-param +++ b/tests/ref/fate/api-mjpeg-codec-param @@ -136,6 +136,7 @@ stream=0, decode=0 pixel_format=yuvj422p video_size=400x225 max_pixels=2147483647 + max_samples=2147483647 hwaccel_flags=0x00000001 extra_hw_frames=-1 discard_damaged_percentage=95 @@ -277,6 +278,7 @@ stream=0, decode=1 pixel_format=yuvj422p video_size=400x225 max_pixels=2147483647 + max_samples=2147483647 hwaccel_flags=0x00000001 extra_hw_frames=-1 discard_damaged_percentage=95 diff --git a/tests/ref/fate/api-png-codec-param b/tests/ref/fate/api-png-codec-param index a47d0963da..c04c8cc7c1 100644 --- a/tests/ref/fate/api-png-codec-param +++ b/tests/ref/fate/api-png-codec-param @@ -136,6 +136,7 @@ stream=0, decode=0 pixel_format=rgba video_size=128x128 max_pixels=2147483647 + max_samples=2147483647 hwaccel_flags=0x00000001 extra_hw_frames=-1 discard_damaged_percentage=95 @@ -277,6 +278,7 @@ stream=0, decode=1 pixel_format=rgba video_size=128x128 max_pixels=2147483647 + max_samples=2147483647 hwaccel_flags=0x00000001 extra_hw_frames=-1 discard_damaged_percentage=95
TODO: APIChanges, bump version Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/avcodec.h | 8 ++++++++ libavcodec/options_table.h | 1 + tests/ref/fate/api-mjpeg-codec-param | 2 ++ tests/ref/fate/api-png-codec-param | 2 ++ 4 files changed, 13 insertions(+)