| Message ID | CAJiLW2Oo=i+b-Dhg+PWg_Wiu5ioefcSecSOziYZfW9zosXXQhA@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 21 November 2016 at 20:44, Martin Vignali <martin.vignali@gmail.com> wrote: > Hello, > > New patchs in attach. > Correction have been made followings comments from Andreas and Carl. > > @Rotislav : thanks for your answer. > > Martin > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > I don't think you need the checks, it was pointed out that ff_set_dimensions already does that. Also IIRC lavc doesn't support widths or heights over 16384 anyway. >unsigned int pixel_size Please use uint32_t instead of unsigned ints throughout the patches.. I'm paranoid about unspecified sizes.
Hi, On Mon, Nov 21, 2016 at 4:40 PM, Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > On 21 November 2016 at 20:44, Martin Vignali <martin.vignali@gmail.com> > wrote: > > > Hello, > > > > New patchs in attach. > > Correction have been made followings comments from Andreas and Carl. > > > > @Rotislav : thanks for your answer. > > > > Martin > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > I don't think you need the checks, it was pointed out that > ff_set_dimensions already does that. > Also IIRC lavc doesn't support widths or heights over 16384 anyway. > > >unsigned int pixel_size > Please use uint32_t instead of unsigned ints throughout the patches.. I'm > paranoid about unspecified sizes. I don't think we typically encourage that, except for things that are arrays... Ronald
On 21 November 2016 at 21:47, Ronald S. Bultje <rsbultje@gmail.com> wrote: > Hi, > > On Mon, Nov 21, 2016 at 4:40 PM, Rostislav Pehlivanov <atomnuker@gmail.com > > > wrote: > > > On 21 November 2016 at 20:44, Martin Vignali <martin.vignali@gmail.com> > > wrote: > > > > > Hello, > > > > > > New patchs in attach. > > > Correction have been made followings comments from Andreas and Carl. > > > > > > @Rotislav : thanks for your answer. > > > > > > Martin > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > I don't think you need the checks, it was pointed out that > > ff_set_dimensions already does that. > > Also IIRC lavc doesn't support widths or heights over 16384 anyway. > > > > >unsigned int pixel_size > > Please use uint32_t instead of unsigned ints throughout the patches.. I'm > > paranoid about unspecified sizes. > > > I don't think we typically encourage that, except for things that are > arrays... > > Ronald > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Didn't know that, ignore it. Still though, I don't think there's a need to check the dimensions.
On Mon, Nov 21, 2016 at 21:44:55 +0100, Martin Vignali wrote: > + avpriv_request_sample(s->avctx, "ZIP without predictor compression"); > + return AVERROR_PATCHWELCOME; > + break; > + case 3: > + avpriv_request_sample(s->avctx, "ZIP with predictor compression"); > + return AVERROR_PATCHWELCOME; > + break; Are these the only two (detectable) unsupported features? I.e. all other error messages are true errors, not "unsupported"? Just wondering, Moritz
Hello, 2016-11-21 23:16 GMT+01:00 Moritz Barsnick <barsnick@gmx.net>: > On Mon, Nov 21, 2016 at 21:44:55 +0100, Martin Vignali wrote: > > + avpriv_request_sample(s->avctx, "ZIP without predictor > compression"); > > + return AVERROR_PATCHWELCOME; > > + break; > > + case 3: > > + avpriv_request_sample(s->avctx, "ZIP with predictor > compression"); > > + return AVERROR_PATCHWELCOME; > > + break; > > Are these the only two (detectable) unsupported features? I.e. all > other error messages are true errors, not "unsupported"? > <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > I think, i use AVERROR_PATCHWELCOME, for each documented features, that the decoder does not support. Do you have a specific place where you think i return the wrong message ? I mainly use this link, for the psd specification : https://www.adobe.com/devnet-apps/photoshop/fileformatashtml/ @Rostislav : > I don't think you need the checks, it was pointed out that > ff_set_dimensions already does that. > Also IIRC lavc doesn't support widths or heights over 16384 anyway. Andreas ask why i choose 30 000 for check widths/heights And the choice come from the Adobe documentation : "Supported range is 1 to 30,000." But i doesn't know about the 16384 limit, in lavc. I wrongly suppose, it was int max So if this is the real limit value is under 30 000, you're right, the check is not need. Martin
On Tue, Nov 22, 2016 at 12:15:16AM +0100, Martin Vignali wrote: > Hello, > > 2016-11-21 23:16 GMT+01:00 Moritz Barsnick <barsnick@gmx.net>: > > > On Mon, Nov 21, 2016 at 21:44:55 +0100, Martin Vignali wrote: > > > + avpriv_request_sample(s->avctx, "ZIP without predictor > > compression"); > > > + return AVERROR_PATCHWELCOME; > > > + break; > > > + case 3: > > > + avpriv_request_sample(s->avctx, "ZIP with predictor > > compression"); > > > + return AVERROR_PATCHWELCOME; > > > + break; > > > > Are these the only two (detectable) unsupported features? I.e. all > > other error messages are true errors, not "unsupported"? > > <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > > > I think, i use AVERROR_PATCHWELCOME, for each documented features, that > the decoder does not support. > Do you have a specific place where you think i return the wrong message ? > > I mainly use this link, for the psd specification : > https://www.adobe.com/devnet-apps/photoshop/fileformatashtml/ > > > @Rostislav : > > I don't think you need the checks, it was pointed out that > > ff_set_dimensions already does that. > > Also IIRC lavc doesn't support widths or heights over 16384 anyway. > > Andreas ask why i choose 30 000 for check widths/heights > And the choice come from the Adobe documentation : "Supported range is 1 to > 30,000." > > But i doesn't know about the 16384 limit, in lavc. I wrongly suppose, it > was int max ./ffmpeg -i matrixbench_mpeg2.mpg -vf scale=60000:16 -vcodec ffv1 test.nut works fine [...]
From df98013ba80689fdd6f9f8074b23b64c915a1f2c Mon Sep 17 00:00:00 2001 From: Martin Vignali <martin.vignali@gmail.com> Date: Mon, 21 Nov 2016 21:39:23 +0100 Subject: [PATCH 2/2] libavformat : add Photoshop PSD demuxer. --- libavformat/Makefile | 1 + libavformat/allformats.c | 1 + libavformat/img2dec.c | 32 ++++++++++++++++++++++++++++++++ libavformat/version.h | 2 +- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/libavformat/Makefile b/libavformat/Makefile index f93658d..f803d04 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -235,6 +235,7 @@ OBJS-$(CONFIG_IMAGE_PGM_PIPE_DEMUXER) += img2dec.o img2.o OBJS-$(CONFIG_IMAGE_PICTOR_PIPE_DEMUXER) += img2dec.o img2.o OBJS-$(CONFIG_IMAGE_PNG_PIPE_DEMUXER) += img2dec.o img2.o OBJS-$(CONFIG_IMAGE_PPM_PIPE_DEMUXER) += img2dec.o img2.o +OBJS-$(CONFIG_IMAGE_PSD_PIPE_DEMUXER) += img2dec.o img2.o OBJS-$(CONFIG_IMAGE_QDRAW_PIPE_DEMUXER) += img2dec.o img2.o OBJS-$(CONFIG_IMAGE_SGI_PIPE_DEMUXER) += img2dec.o img2.o OBJS-$(CONFIG_IMAGE_SUNRAST_PIPE_DEMUXER) += img2dec.o img2.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 6a216ef..9d77b9c 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -366,6 +366,7 @@ void av_register_all(void) REGISTER_DEMUXER (IMAGE_PICTOR_PIPE, image_pictor_pipe); REGISTER_DEMUXER (IMAGE_PNG_PIPE, image_png_pipe); REGISTER_DEMUXER (IMAGE_PPM_PIPE, image_ppm_pipe); + REGISTER_DEMUXER (IMAGE_PSD_PIPE, image_psd_pipe); REGISTER_DEMUXER (IMAGE_QDRAW_PIPE, image_qdraw_pipe); REGISTER_DEMUXER (IMAGE_SGI_PIPE, image_sgi_pipe); REGISTER_DEMUXER (IMAGE_SUNRAST_PIPE, image_sunrast_pipe); diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c index a920f46..f1a0e7f 100644 --- a/libavformat/img2dec.c +++ b/libavformat/img2dec.c @@ -822,6 +822,37 @@ static int png_probe(AVProbeData *p) return 0; } +static int psd_probe(AVProbeData *p) +{ + const uint8_t *b = p->buf; + int ret = 0; + uint16_t color_mode; + + if (AV_RL32(b) == MKTAG('8','B','P','S')) { + ret += 1; + } else { + return 0; + } + + if ((b[4] == 0) && (b[5] == 1)) {/* version 1 is PSD, version 2 is PSB */ + ret += 1; + } else { + return 0; + } + + if ((AV_RL32(b+6) == 0) && (AV_RL16(b+10) == 0))/* reserved must be 0 */ + ret += 1; + + color_mode = AV_RB16(b+24); + if ((color_mode <= 9) && (color_mode != 5) && (color_mode != 6)) + ret += 1; + + if (ret) + return AVPROBE_SCORE_EXTENSION + ret; + + return 0; +} + static int sgi_probe(AVProbeData *p) { const uint8_t *b = p->buf; @@ -947,6 +978,7 @@ IMAGEAUTO_DEMUXER(pgmyuv, AV_CODEC_ID_PGMYUV) IMAGEAUTO_DEMUXER(pictor, AV_CODEC_ID_PICTOR) IMAGEAUTO_DEMUXER(png, AV_CODEC_ID_PNG) IMAGEAUTO_DEMUXER(ppm, AV_CODEC_ID_PPM) +IMAGEAUTO_DEMUXER(psd, AV_CODEC_ID_PSD) IMAGEAUTO_DEMUXER(qdraw, AV_CODEC_ID_QDRAW) IMAGEAUTO_DEMUXER(sgi, AV_CODEC_ID_SGI) IMAGEAUTO_DEMUXER(sunrast, AV_CODEC_ID_SUNRAST) diff --git a/libavformat/version.h b/libavformat/version.h index 24a7534..192b790 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -32,7 +32,7 @@ // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium) // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 57 -#define LIBAVFORMAT_VERSION_MINOR 58 +#define LIBAVFORMAT_VERSION_MINOR 59 #define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ -- 1.9.3 (Apple Git-50)