| Message ID | 20180717193919.99925-2-vittorio.giovara@gmail.com |
|---|---|
| State | New |
| Headers | show |
2018-07-17 21:39 GMT+02:00, Vittorio Giovara <vittorio.giovara@gmail.com>:
> YUV410P requires that sizes are divisible by 4.
Do you mean that AV_PIX_FMT_YUV410P requires it?
Where is this documented?
Carl Eugen
On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2018-07-17 21:39 GMT+02:00, Vittorio Giovara <vittorio.giovara@gmail.com>: > > YUV410P requires that sizes are divisible by 4. > > Do you mean that AV_PIX_FMT_YUV410P requires it? > Where is this documented? > Its a consequence of the subsampling factor. 4:1:0 is one-fourth the horizontal resolution and half the vertical resolution, as such the width has to be a multiple of 4 for that to result in a valid chroma dimension. - Hendrik
2018-07-17 23:58 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> > wrote: >> >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara <vittorio.giovara@gmail.com>: >> > YUV410P requires that sizes are divisible by 4. >> >> Do you mean that AV_PIX_FMT_YUV410P requires it? >> Where is this documented? > > Its a consequence of the subsampling factor. 4:1:0 is one-fourth the > horizontal resolution and half the vertical resolution, as such the > width has to be a multiple of 4 for that to result in a valid chroma > dimension. (Apart from the fact that it appears to work here and is needed to archive some multimedia files.) Why wouldn't the chroma dimension be rounded up? Carl Eugen
On Wed, Jul 18, 2018 at 12:13 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2018-07-17 23:58 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: > > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> > > wrote: > >> > >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara <vittorio.giovara@gmail.com>: > >> > YUV410P requires that sizes are divisible by 4. > >> > >> Do you mean that AV_PIX_FMT_YUV410P requires it? > >> Where is this documented? > > > > Its a consequence of the subsampling factor. 4:1:0 is one-fourth the > > horizontal resolution and half the vertical resolution, as such the > > width has to be a multiple of 4 for that to result in a valid chroma > > dimension. > > (Apart from the fact that it appears to work here and is needed to > archive some multimedia files.) > Why wouldn't the chroma dimension be rounded up? > Where is it ever specified that this would be the case? If I'm an API user, and I get an odd image size with such a subsampling factor, what guarantees do I have that the chroma plane is big enough to be rounded up, and I don't overread the buffer, or process garbage information? Its the same with 4:2:0 images, non-mod2 images using 4:2:0 are just flat out invalid, or non-mod4 if they are interlaced even. Thats why cropping info exists. You can store odd-sized images like that if you really want to by padding it, but you need to crop it after converting it to RGB or another format without such limitations. This doesn't change how the format is stored, just when cropping is applied. - Hendrik
2018-07-18 0:22 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: > On Wed, Jul 18, 2018 at 12:13 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> > wrote: >> >> 2018-07-17 23:58 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: >> > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> >> > wrote: >> >> >> >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara >> >> <vittorio.giovara@gmail.com>: >> >> > YUV410P requires that sizes are divisible by 4. >> >> >> >> Do you mean that AV_PIX_FMT_YUV410P requires it? >> >> Where is this documented? >> > >> > Its a consequence of the subsampling factor. 4:1:0 is one-fourth the >> > horizontal resolution and half the vertical resolution, as such the >> > width has to be a multiple of 4 for that to result in a valid chroma >> > dimension. >> >> (Apart from the fact that it appears to work here and is needed to >> archive some multimedia files.) >> Why wouldn't the chroma dimension be rounded up? > > Where is it ever specified that this would be the case? Which other options exist? > If I'm an API user, and I get an odd image size with such a > subsampling factor, what guarantees do I have that the chroma plane is > big enough to be rounded up, and I don't overread the buffer, or > process garbage information? The exact same guarantees as for any other buffer FFmpeg provides? > Its the same with 4:2:0 images, non-mod2 images using 4:2:0 are just > flat out invalid, or non-mod4 if they are interlaced even. And such files do not exist or what exactly do you claim? > Thats why cropping info exists. You can store odd-sized images like > that if you really want to by padding it, but you need to crop it > after converting it to RGB or another format without such limitations. But that only works if that's how the encoder did it, no? > This doesn't change how the format is stored, just when cropping is applied. Then I wonder what exactly this patch fixes (if nothing is changed). Carl Eugen
On Wed, Jul 18, 2018 at 12:37 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > This doesn't change how the format is stored, just when cropping is applied. > > Then I wonder what exactly this patch fixes (if nothing is changed). > "Change when cropping is applied" is nothing? Thats everything this patch is about, and plenty to resolve such issues. - Hendrik
On Tue, Jul 17, 2018 at 11:58:06PM +0200, Hendrik Leppkes wrote: > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > 2018-07-17 21:39 GMT+02:00, Vittorio Giovara <vittorio.giovara@gmail.com>: > > > YUV410P requires that sizes are divisible by 4. > > > > Do you mean that AV_PIX_FMT_YUV410P requires it? > > Where is this documented? > > > > Its a consequence of the subsampling factor. this does not follow from what you write below. But more so this is not how pixel formats were implemented in FFmpeg. Where does this idea come from ? > 4:1:0 is one-fourth the > horizontal resolution and half the vertical resolution, as such the > width has to be a multiple of 4 for that to result in a valid chroma > dimension. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Wed, Jul 18, 2018 at 12:22:21AM +0200, Hendrik Leppkes wrote: > On Wed, Jul 18, 2018 at 12:13 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > 2018-07-17 23:58 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: > > > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> > > > wrote: > > >> > > >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara <vittorio.giovara@gmail.com>: > > >> > YUV410P requires that sizes are divisible by 4. > > >> > > >> Do you mean that AV_PIX_FMT_YUV410P requires it? > > >> Where is this documented? > > > > > > Its a consequence of the subsampling factor. 4:1:0 is one-fourth the > > > horizontal resolution and half the vertical resolution, as such the > > > width has to be a multiple of 4 for that to result in a valid chroma > > > dimension. > > > > (Apart from the fact that it appears to work here and is needed to > > archive some multimedia files.) > > Why wouldn't the chroma dimension be rounded up? > > > > Where is it ever specified that this would be the case? > If I'm an API user, and I get an odd image size with such a > subsampling factor, what guarantees do I have that the chroma plane is > big enough to be rounded up, and I don't overread the buffer, or > process garbage information? This is a good question. I would argue that logic gurantees this, because without the chroma plane being large enough you would have vissible areas with luma samples but no chroma samples. But besides logic, i think all of our code works this way, FFmpeg itself would overead if this wasnt true > > Its the same with 4:2:0 images, non-mod2 images using 4:2:0 are just > flat out invalid, or non-mod4 if they are interlaced even. But this is not really true. There are many files that have mod2 != 0 with 420. For example this is common in jpegs. gimp for example will happily create such files you just crop or scale and you have a 50% chance to get that on export [...]
On Tue, Jul 17, 2018 at 11:58:06PM +0200,Michael Niedermayer <michaelat niedermayer.cc> wrote: > > Its a consequence of the subsampling factor. > > this does not follow from what you write below. But more so this is not > how pixel formats were implemented in FFmpeg. > Where does this idea come from ? I found the following description of this pixel format pretty accurate: https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html I am not sure how non-mod4 sizes work, but probably codecs within ffmpeg take into account more alignment than necessary and make things work within the ffmpeg libraries. Problems like this usually affect API users. Vittorio
On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara <vittorio.giovara@gmail.com> wrote: >> this does not follow from what you write below. But more so this is not >> how pixel formats were implemented in FFmpeg. >> Where does this idea come from ? > > I found the following description of this pixel format pretty accurate: > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html > > I am not sure how non-mod4 sizes work, but probably codecs within ffmpeg > take into account more alignment than necessary and make things work To expand on this (and other replies): The behavior in FFmpeg is very unexpected for an API user who may wish to actually use the returned yuv410p data with an application or other library that is *not* from FFmpeg, such as a renderer, or external encoder lib, resizer, etc. Everything else on the planet assumes that if you have a buffer of a specific chroma subsampling type, you actually have enough data for it to be valid, with width/height that make it so. It's very surprising when you get back a set of buffers/width/height that don't make sense for a given pixel format, and is little to no documentation about why/how. I don't think "logic guarantees the buffer is mod4/aligned" is a reasonable thing to tell an API user in lieu of documentation for unexpected behavior. Cheers, - Derek
On Wed, Jul 18, 2018 at 11:03:47AM -0400, Derek Buitenhuis wrote: > On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara > <vittorio.giovara@gmail.com> wrote: > >> this does not follow from what you write below. But more so this is not > >> how pixel formats were implemented in FFmpeg. > >> Where does this idea come from ? > > > > I found the following description of this pixel format pretty accurate: > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html > > > > I am not sure how non-mod4 sizes work, but probably codecs within ffmpeg > > take into account more alignment than necessary and make things work > > To expand on this (and other replies): The behavior in FFmpeg is very unexpected > for an API user who may wish to actually use the returned yuv410p data with an > application or other library that is *not* from FFmpeg, such as a renderer, or > external encoder lib, resizer, etc. Everything else on the planet > assumes that if > you have a buffer of a specific chroma subsampling type, you actually > have enough > data for it to be valid, with width/height that make it so. It's very > surprising when > you get back a set of buffers/width/height that don't make sense for a > given pixel > format, and is little to no documentation about why/how. I think i see what you mean. But iam not sure i understand how the proposed changes would be the ideal solution. For example: lets assume we have a 3x3 image of 420 or 410 (be that from a jpeg or whatever) If we want to use this with software that that does support odd resolutions then it should just work. Theres no 4th column or row in the logic image that could be used. OTOH if we want to use this with softwate that does not support odd resolutions then its not going to work with a 3x3 (as odd) or 2x2 (looses a column and row) or a 4x4 (which has a column and a row that is uninitialized or black) what i mean is that the API by which one exports the width and height doesnt really affect this. To get from a 3x3 to something that actually works with external code which only supports even resolutions, something somewhere has to make it even and either scale, crop or fill in. More specifically, saying that this 3x3 is a 4x4 image with crop is not really true as theres not neccesarily any data in the last column and row. And a filter or encoder using that anyway could produce bad output > > I don't think "logic guarantees the buffer is mod4/aligned" is a > reasonable thing > to tell an API user in lieu of documentation for unexpected behavior. just posted a small patch to document this [...]
On Wed, Jul 18, 2018 at 10:37 PM, Michael Niedermayer < michael@niedermayer.cc> wrote: > On Wed, Jul 18, 2018 at 11:03:47AM -0400, Derek Buitenhuis wrote: > > On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara > > <vittorio.giovara@gmail.com> wrote: > > >> this does not follow from what you write below. But more so this is > not > > >> how pixel formats were implemented in FFmpeg. > > >> Where does this idea come from ? > > > > > > I found the following description of this pixel format pretty accurate: > > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html > > > > > > I am not sure how non-mod4 sizes work, but probably codecs within > ffmpeg > > > take into account more alignment than necessary and make things work > > > > To expand on this (and other replies): The behavior in FFmpeg is very > unexpected > > for an API user who may wish to actually use the returned yuv410p data > with an > > application or other library that is *not* from FFmpeg, such as a > renderer, or > > external encoder lib, resizer, etc. Everything else on the planet > > assumes that if > > you have a buffer of a specific chroma subsampling type, you actually > > have enough > > data for it to be valid, with width/height that make it so. It's very > > surprising when > > you get back a set of buffers/width/height that don't make sense for a > > given pixel > > format, and is little to no documentation about why/how. > > I think i see what you mean. > But iam not sure i understand how the proposed changes would be the ideal > solution. > The change would guarantee that the received buffer respects the pixel format constraints, and informs the users that there are some lines and columns that ought to be cropped away in order to display the desired (or valid) output. > For example: > lets assume we have a 3x3 image of 420 or 410 (be that from a jpeg or > whatever) > > If we want to use this with software that that does support odd resolutions > then it should just work. Theres no 4th column or row in the logic image > that > could be used. > > OTOH if we want to use this with softwate that does not support odd > resolutions > then its not going to work with a 3x3 (as odd) or 2x2 (looses a column and > row) > or a 4x4 (which has a column and a row that is uninitialized or black) > The problem with this reasoning is that things "happen" to work only because ffmpeg decoders always over-allocate the output buffers and make sure that there is addressable data. So as long as you stay "within" the ffmpeg APIs, everything is perfect, however when you use such data in third party libraries you risk running in these issues. > what i mean is that the API by which one exports the width and height > doesnt > really affect this. To get from a 3x3 to something that actually works with > external code which only supports even resolutions, something somewhere has > to make it even and either scale, crop or fill in. > Well, IMO the API is low level enough that it would make sense to always return the coded sizes and let the user crop to the desired resolution. Yes several filters/encoders and user apps would need some tweaks, but since this is the n-th case in which cropping produces broken results maybe it's worth it. > More specifically, saying that this 3x3 is a 4x4 image with crop is not > really > true as theres not neccesarily any data in the last column and row. And a > filter or encoder using that anyway could produce bad output > There is always data in the last column and row, or it would have been impossible to encode the buffer. Whether it contains visible data or garbage is another story. > I don't think "logic guarantees the buffer is mod4/aligned" is a > > reasonable thing > > to tell an API user in lieu of documentation for unexpected behavior. > > just posted a small patch to document this > I added some padding/cropping code to my program, so that's enough for now, but i think a more thorough solution should be found.
On Thu, Jul 19, 2018 at 05:06:09PM +0200, Vittorio Giovara wrote: > On Wed, Jul 18, 2018 at 10:37 PM, Michael Niedermayer < > michael@niedermayer.cc> wrote: > > > On Wed, Jul 18, 2018 at 11:03:47AM -0400, Derek Buitenhuis wrote: > > > On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara > > > <vittorio.giovara@gmail.com> wrote: > > > >> this does not follow from what you write below. But more so this is > > not > > > >> how pixel formats were implemented in FFmpeg. > > > >> Where does this idea come from ? > > > > > > > > I found the following description of this pixel format pretty accurate: > > > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html > > > > > > > > I am not sure how non-mod4 sizes work, but probably codecs within > > ffmpeg > > > > take into account more alignment than necessary and make things work > > > > > > To expand on this (and other replies): The behavior in FFmpeg is very > > unexpected > > > for an API user who may wish to actually use the returned yuv410p data > > with an > > > application or other library that is *not* from FFmpeg, such as a > > renderer, or > > > external encoder lib, resizer, etc. Everything else on the planet > > > assumes that if > > > you have a buffer of a specific chroma subsampling type, you actually > > > have enough > > > data for it to be valid, with width/height that make it so. It's very > > > surprising when > > > you get back a set of buffers/width/height that don't make sense for a > > > given pixel > > > format, and is little to no documentation about why/how. > > > > I think i see what you mean. > > But iam not sure i understand how the proposed changes would be the ideal > > solution. > > > > The change would guarantee that the received buffer respects the pixel > format constraints, I think there is possibly some disagreement on what the "pixel format constraints" are or should be, judging from the messages here > and informs the users that there are some lines and > columns that ought to be cropped away in order to display the desired (or > valid) output. > > > > For example: > > lets assume we have a 3x3 image of 420 or 410 (be that from a jpeg or > > whatever) > > > > If we want to use this with software that that does support odd resolutions > > then it should just work. Theres no 4th column or row in the logic image > > that > > could be used. > > > > OTOH if we want to use this with softwate that does not support odd > > resolutions > > then its not going to work with a 3x3 (as odd) or 2x2 (looses a column and > > row) > > or a 4x4 (which has a column and a row that is uninitialized or black) > > > > The problem with this reasoning is that things "happen" to work only > because ffmpeg decoders always over-allocate the output buffers and make > sure that there is addressable data. So as long as you stay "within" the > ffmpeg APIs, everything is perfect, however when you use such data in third > party libraries you risk running in these issues. I read this paragraph twice but i dont think this really says a word about what "the problem" is. it just asserts that there is one > > > > what i mean is that the API by which one exports the width and height > > doesnt > > really affect this. To get from a 3x3 to something that actually works with > > external code which only supports even resolutions, something somewhere has > > to make it even and either scale, crop or fill in. > > > > Well, IMO the API is low level enough that it would make sense to always > return the coded sizes and let the user crop to the desired resolution. Thats not how most decoders work currently in FFmpeg. Changing the behavior of 2 decoders out of many for one pixel format out of many is certainly not a good idea no matter on which side of the argument one is on. More so this is a change in behavior which should be discussed on ffmpeg-devel and not just "done" by a patch to some decoders. I mean this is the wrong thread to discuss if we want to make such a change. A thread to discuss it should make clear in its title what it is about and not be specific to a single decoder > Yes several filters/encoders and user apps would need some tweaks, but > since this is the n-th case in which cropping produces broken results maybe > it's worth it. > > > > More specifically, saying that this 3x3 is a 4x4 image with crop is not > > really > > true as theres not neccesarily any data in the last column and row. And a > > filter or encoder using that anyway could produce bad output > > > > There is always data in the last column and row, or it would have been > impossible to encode the buffer. Taken litterally, what you say would mean that i could not write (on paper or in a file) 5 8 9 1 2 3 9 9 9 1 2 2 3 1 1 2 2 as 5 8 9 1 2 3 9 9 9 1 2 2 3 1 1 2 2 but would have to store non existing samples to pad to 4x4 That doesnt make any sense. So i guess you meant something else, but its not really clear to me what you meant > Whether it contains visible data or garbage is another story. > > > I don't think "logic guarantees the buffer is mod4/aligned" is a > > > reasonable thing > > > to tell an API user in lieu of documentation for unexpected behavior. > > > > just posted a small patch to document this > > > > I added some padding/cropping code to my program, so that's enough for now, > but i think a more thorough solution should be found. > -- > Vittorio > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index 7658a51685..8ff0f0deb8 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -971,6 +971,13 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac if ((ret = av_frame_ref(data, f->picture.f)) < 0) return ret; + if (p->format == AV_PIX_FMT_YUV410P) { + p->crop_left = 0; + p->crop_top = 0; + p->crop_right = FFALIGN(p->width, 4) - p->width; + p->crop_bottom = FFALIGN(p->height, 4) - p->height; + } + *got_frame = 1; return buf_size; @@ -1091,5 +1098,5 @@ AVCodec ff_ffv1_decoder = { .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context), .capabilities = AV_CODEC_CAP_DR1 /*| AV_CODEC_CAP_DRAW_HORIZ_BAND*/ | AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS, - .caps_internal = FF_CODEC_CAP_INIT_CLEANUP + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP | FF_CODEC_CAP_EXPORTS_CROPPING, };