On Mon, Jun 13, 2016 at 08:44:36PM +0200, Jerome Martinez wrote:
> Le 13/06/2016 à 19:55, Michael Niedermayer a écrit :
> >On Mon, Jun 13, 2016 at 07:31:15PM +0200, Jerome Martinez wrote:
> >>FFV1 decoder:
> >>
> >>When checking pix_fmt mapping, some bitstreams are mapped to an
> >>incorrect pix_fmt instead of being rejected (ENOSYS).
> >>Actually, such bitstreams are not supported (FFmpeg encoder does not
> >>produce such bitstream, such bitstream may come only from another
> >>encoder for the moment).
> >>
> >>- JPEG 2000 RCT 11/13/15/16 bit depths are mapped to a 8-bit FFmpeg
> >>pix_fmt (e.g. bgr0), which is not expected.
> >>- JPEG 2000 RCT 9/10/12/14 bit depths with alpha are mapped to a
> >>FFmpeg pix_fmt without alpha (e.g. AV_PIX_FMT_GBRP9 for 9-bit with
> >>alpha), which is not expected.
> >>
> >>The order for choosing the pix_fmt is changed to the one used by
> >>YCbCr selection (<=8 bit first).
> >>" && !f->transparency" is added to the other lines.
> >>
> >> ffv1dec.c | 15 ++++++++-------
> >> 1 file changed, 8 insertions(+), 7 deletions(-)
> >>27efc1a9821576a2a5bd8d1feaaf6c584fc69a62
> >>0001-avcodec-ffv1dec-fix-some-unsupported-pix_fmt.patch
> >> From 90bfd748b0e25d7a0be037280f4a0a40242f8d27 Mon Sep 17 00:00:00 2001
> >>From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jer...@mediaarea.net>
> >>Date: Mon, 13 Jun 2016 19:18:22 +0200
> >>Subject: [PATCH] avcodec/ffv1dec: fix some unsupported pix_fmt
> >>MIME-Version: 1.0
> >>Content-Type: text/plain; charset=UTF-8
> >>Content-Transfer-Encoding: 8bit
> >>
> >>Signed-off-by: Jérôme Martinez <jer...@mediaarea.net>
> >>---
> >> libavcodec/ffv1dec.c | 15 ++++++++-------
> >> 1 file changed, 8 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> >>index d2bf3a8..6a932b2 100644
> >>--- a/libavcodec/ffv1dec.c
> >>+++ b/libavcodec/ffv1dec.c
> >>@@ -768,17 +768,18 @@ static int read_header(FFV1Context *f)
> >> "chroma subsampling not supported in this
> >> colorspace\n");
> >> return AVERROR(ENOSYS);
> >> }
> >>- if ( f->avctx->bits_per_raw_sample == 9)
> >>+ if ( f->avctx->bits_per_raw_sample <= 8 && !f->transparency)
> >>+ f->avctx->pix_fmt = AV_PIX_FMT_0RGB32;
> >>+ else if (f->avctx->bits_per_raw_sample <= 8 && f->transparency)
> >>+ f->avctx->pix_fmt = AV_PIX_FMT_RGB32;
> >>+ else if (f->avctx->bits_per_raw_sample == 9 && !f->transparency)
> >> f->avctx->pix_fmt = AV_PIX_FMT_GBRP9;
> >>- else if (f->avctx->bits_per_raw_sample == 10)
> >>+ else if (f->avctx->bits_per_raw_sample == 10 && !f->transparency)
> >> f->avctx->pix_fmt = AV_PIX_FMT_GBRP10;
> >>- else if (f->avctx->bits_per_raw_sample == 12)
> >>+ else if (f->avctx->bits_per_raw_sample == 12 && !f->transparency)
> >> f->avctx->pix_fmt = AV_PIX_FMT_GBRP12;
> >>- else if (f->avctx->bits_per_raw_sample == 14)
> >>+ else if (f->avctx->bits_per_raw_sample == 14 && !f->transparency)
> >> f->avctx->pix_fmt = AV_PIX_FMT_GBRP14;
> >>- else
> >>- if (f->transparency) f->avctx->pix_fmt = AV_PIX_FMT_RGB32;
> >>- else f->avctx->pix_fmt = AV_PIX_FMT_0RGB32;
> >an else "error" feels missing
>
> I did exactly as the code few lines above: if f->colorspace == 0 and
> f->avctx->bits_per_raw_sample >=17, there is also no else:
> } else if (f->avctx->bits_per_raw_sample == 16 && f->transparency){
> switch(16 * f->chroma_h_shift + f->chroma_v_shift) {
> case 0x00: f->avctx->pix_fmt = AV_PIX_FMT_YUVA444P16; break;
> case 0x10: f->avctx->pix_fmt = AV_PIX_FMT_YUVA422P16; break;
> case 0x11: f->avctx->pix_fmt = AV_PIX_FMT_YUVA420P16; break;
> }
> } // **** an else "error" feels missing here too. ****
>
>
>
> >pix_fmt would not be initialized after the patch
>
> When I test the code, pix_fmt is initialized to -1 (AV_PIX_FMT_NONE)
> before running this part, and the error is caught by the "format not
> supported" message, which looks like a good message for that.
>
>
> I have no problem with adding such else in the patch, but the code
> would not be coherent (an else "error" feels missing if
> f->colorspace == 0, and else "error" does not feel missing if
> f->colorspace == 1), so I would like to have the confirmation that
> this is what you really want.
>
> Note: BTW about missing things, there is no av_log() message when
> f->colorspace == 0 && f->transparency && !f->chroma_plane &&
> ->avctx->bits_per_raw_sample >8 (=gray+alpha with a bit depth of
> more than 8), just a "else return AVERROR(ENOSYS);", I was actually
> willing to remove these 2 lines in order to have more coherency (no
> "else" so "format not supported" message a bit later in the code)
> but I did not want to add different fixes in the same patch.ok patch applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
