Re: [FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected
On Thu, Jul 19, 2018 at 05:06:09PM +0200, Vittorio Giovara wrote: > On Wed, Jul 18, 2018 at 10:37 PM, Michael Niedermayer < > mich...@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 > > > 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
Re: [FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected
On Wed, Jul 18, 2018 at 10:37 PM, Michael Niedermayer < mich...@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 > > 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. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected
On Wed, Jul 18, 2018 at 11:03:47AM -0400, Derek Buitenhuis wrote: > On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara > 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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected
On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected
On Tue, Jul 17, 2018 at 11:58:06PM +0200,Michael Niedermayer 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
