Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
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 wrote: > > > > 2018-07-17 21:39 GMT+02:00, Vittorio Giovara : > > > 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 -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Old school: Use the lowest level language in which you can solve the problem conveniently. New school: Use the highest level language in which the latest supercomputer can solve the problem without the user falling asleep waiting. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
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 wrote: > > > > 2018-07-17 23:58 GMT+02:00, Hendrik Leppkes : > > > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos > > > wrote: > > >> > > >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara > > >> : > > >> > 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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
On Wed, Jul 18, 2018 at 12:37 AM Carl Eugen Hoyos 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
2018-07-18 0:22 GMT+02:00, Hendrik Leppkes : > On Wed, Jul 18, 2018 at 12:13 AM Carl Eugen Hoyos > wrote: >> >> 2018-07-17 23:58 GMT+02:00, Hendrik Leppkes : >> > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos >> > wrote: >> >> >> >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara >> >> : >> >> > 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
On Wed, Jul 18, 2018 at 12:13 AM Carl Eugen Hoyos wrote: > > 2018-07-17 23:58 GMT+02:00, Hendrik Leppkes : > > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos > > wrote: > >> > >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara : > >> > 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
2018-07-17 23:58 GMT+02:00, Hendrik Leppkes : > On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos > wrote: >> >> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara : >> > 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
On Tue, Jul 17, 2018 at 11:54 PM Carl Eugen Hoyos wrote: > > 2018-07-17 21:39 GMT+02:00, Vittorio Giovara : > > 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
2018-07-17 21:39 GMT+02:00, Vittorio Giovara : > YUV410P requires that sizes are divisible by 4. Do you mean that AV_PIX_FMT_YUV410P requires it? Where is this documented? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected
YUV410P requires that sizes are divisible by 4. There are some encoders
(including ffmpeg's) that ignore this constraint and encode a different
value in the bitstream itself. Handle that case by exporting the relative
cropping information.
---
Alternatively it is possible to always enforce mod4 sizes and call
it a day. I haven't explored ways to fix the encoder (or if other
decoders are affected).
Vittorio
libavcodec/ffv1dec.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
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,
};
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
