| Message ID | 20190317212934.18060-1-velocityra@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 3/17/19, velocityra@gmail.com <velocityra@gmail.com> wrote: > From: Nick Renieris <velocityra@gmail.com> > > Prints "DNG images are not supported" if it finds a TIFF image with > the 'DNGVersion' tag. In DNG images with the .tiff extension it, > solves the issue where the TIFF thumbnail in IFD 0 was incorrectly > parsed (related confusion: [1]). Adds an option (-dng_thumbnail) > for dumping TIFF thumbnails, should the user want to. Also prints > the DNG version of the file on the debug channel. > > Additionally: > - Renamed TIFF_WHITE_LEVEL to DNG_WHITE_LEVEL since it is specified > in the DNG spec. > - Added/changed some comments to be more precise in differentiating > between TIFF, TIFF/EP and DNG values. > > Related to ticket: https://trac.ffmpeg.org/ticket/4364 > > --- > > [1]: > https://superuser.com/questions/546879/creating-video-from-dng-images-with-ffmpeg > > Signed-off-by: Nick Renieris <velocityra@gmail.com> > --- > libavcodec/tiff.c | 29 ++++++++++++++++++++++++++++- > libavcodec/tiff.h | 18 +++++++++++++----- > libavformat/img2.c | 1 + > 3 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c > index 112f5b52f4..341c280736 100644 > --- a/libavcodec/tiff.c > +++ b/libavcodec/tiff.c > @@ -54,7 +54,9 @@ typedef struct TiffContext { > AVCodecContext *avctx; > GetByteContext gb; > > + /** options */ > int get_subimage; > + int get_dng_thumbnail; > > int width, height; > unsigned int bpp, bppcount; > @@ -70,6 +72,7 @@ typedef struct TiffContext { > int fill_order; > uint32_t res[4]; > > + int dng_mode; /** denotes that this is a DNG image */ > int is_bayer; > uint8_t pattern[4]; > unsigned white_level; > @@ -1077,7 +1080,7 @@ static int tiff_decode_tag(TiffContext *s, AVFrame > *frame) > case TIFF_SUB_IFDS: > s->sub_ifd = value; > break; > - case TIFF_WHITE_LEVEL: > + case DNG_WHITE_LEVEL: > s->white_level = value; > break; > case TIFF_CFA_PATTERN_DIM: > @@ -1322,6 +1325,20 @@ static int tiff_decode_tag(TiffContext *s, AVFrame > *frame) > case TIFF_SOFTWARE_NAME: > ADD_METADATA(count, "software", NULL); > break; > + case DNG_VERSION: > + if (count == 4) { > + unsigned int ver[4]; > + ver[0] = ff_tget(&s->gb, type, s->le); > + ver[1] = ff_tget(&s->gb, type, s->le); > + ver[2] = ff_tget(&s->gb, type, s->le); > + ver[3] = ff_tget(&s->gb, type, s->le); > + > + av_log(s->avctx, AV_LOG_DEBUG, "DNG file, version > %u.%u.%u.%u\n", > + ver[0], ver[1], ver[2], ver[3]); > + > + s->dng_mode = 1; > + } > + break; > default: > if (s->avctx->err_recognition & AV_EF_EXPLODE) { > av_log(s->avctx, AV_LOG_ERROR, > @@ -1375,6 +1392,7 @@ again: > s->fill_order = 0; > s->white_level = 0; > s->is_bayer = 0; > + s->dng_mode = 0; > free_geotags(s); > > // Reset these offsets so we can tell if they were set this frame > @@ -1389,6 +1407,14 @@ again: > return ret; > } > > + if (s->dng_mode) { > + av_log(avctx, AV_LOG_ERROR, "DNG images are not supported\n"); > + av_log(avctx, AV_LOG_INFO, "Consider using -dng_thumbnail for > decoding the embedded thumbail instead\n"); > + > + if (!s->get_dng_thumbnail) > + return AVERROR_PATCHWELCOME; > + } > + > if (s->sub_ifd && s->get_subimage) { > off = s->sub_ifd; > if (off >= UINT_MAX - 14 || avpkt->size < off + 14) { > @@ -1607,6 +1633,7 @@ static av_cold int tiff_end(AVCodecContext *avctx) > #define OFFSET(x) offsetof(TiffContext, x) > static const AVOption tiff_options[] = { > { "subimage", "decode subimage instead if available", > OFFSET(get_subimage), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, > AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_VIDEO_PARAM }, > + { "dng_thumbnail", "decode embedded thumbnail for DNG images, if > available", OFFSET(get_dng_thumbnail), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, > AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_VIDEO_PARAM }, > { NULL }, > }; > > diff --git a/libavcodec/tiff.h b/libavcodec/tiff.h > index 4b08650108..3cede299f6 100644 > --- a/libavcodec/tiff.h > +++ b/libavcodec/tiff.h > @@ -20,7 +20,7 @@ > > /** > * @file > - * TIFF tables > + * TIFF constants & data structures > * > * For more information about the TIFF format, check the official docs at: > * http://partners.adobe.com/public/developer/tiff/index.html > @@ -33,7 +33,7 @@ > #include <stdint.h> > #include "tiff_common.h" > > -/** abridged list of TIFF tags */ > +/** abridged list of TIFF and TIFF/EP tags */ > enum TiffTags { > TIFF_SUBFILE = 0xfe, > TIFF_WIDTH = 0x100, > @@ -85,10 +85,17 @@ enum TiffTags { > TIFF_GEO_KEY_DIRECTORY = 0x87AF, > TIFF_GEO_DOUBLE_PARAMS = 0x87B0, > TIFF_GEO_ASCII_PARAMS = 0x87B1, > - TIFF_WHITE_LEVEL = 0xC61D, > }; > > -/** list of TIFF compression types */ > +/** abridged list of DNG tags */ > +enum DngTags > +{ > + DNG_VERSION = 0xC612, > + DNG_BACKWARD_VERSION = 0xC613, > + DNG_WHITE_LEVEL = 0xC61D, > +}; > + > +/** list of TIFF, TIFF/EP and DNG compression types */ > enum TiffCompr { > TIFF_RAW = 1, > TIFF_CCITT_RLE, > @@ -151,6 +158,7 @@ enum TiffGeoTagKey { > TIFF_VERTICAL_UNITS_GEOKEY = 4099 > }; > > +/** list of TIFF, TIFF/AP and DNG PhotometricInterpretation > (TIFF_PHOTOMETRIC) values */ > enum TiffPhotometric { > TIFF_PHOTOMETRIC_NONE = -1, > TIFF_PHOTOMETRIC_WHITE_IS_ZERO, /* mono or grayscale, 0 is white > */ > @@ -163,7 +171,7 @@ enum TiffPhotometric { > TIFF_PHOTOMETRIC_CIE_LAB = 8, /* 1976 CIE L*a*b* */ > TIFF_PHOTOMETRIC_ICC_LAB, /* ICC L*a*b* */ > TIFF_PHOTOMETRIC_ITU_LAB, /* ITU L*a*b* */ > - TIFF_PHOTOMETRIC_CFA = 32803, /* Color Filter Array (DNG) */ > + TIFF_PHOTOMETRIC_CFA = 32803, /* Color Filter Array (TIFF/AP and > DNG) */ > TIFF_PHOTOMETRIC_LOG_L = 32844, /* CIE Log2(L) */ > TIFF_PHOTOMETRIC_LOG_LUV, /* CIE Log L*u*v* */ > TIFF_PHOTOMETRIC_LINEAR_RAW = 34892, /* Linear Raw (DNG) */ > diff --git a/libavformat/img2.c b/libavformat/img2.c > index 8432cc0955..16bc9d2abd 100644 > --- a/libavformat/img2.c > +++ b/libavformat/img2.c > @@ -51,6 +51,7 @@ const IdStrMap ff_img_tags[] = { > { AV_CODEC_ID_TARGA, "tga" }, > { AV_CODEC_ID_TIFF, "tiff" }, > { AV_CODEC_ID_TIFF, "tif" }, > + { AV_CODEC_ID_TIFF, "dng" }, > { AV_CODEC_ID_SGI, "sgi" }, > { AV_CODEC_ID_PTX, "ptx" }, > { AV_CODEC_ID_PCX, "pcx" }, > -- > 2.17.1.windows.2 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Still wrong, You can decode images you linked just fine (albeit with incorrect colors) with command: ffmpeg -subimage 1 -i IMAGE.dng ....rest of command.
Στις Δευ, 18 Μαρ 2019 στις 12:05 π.μ., ο/η Paul B Mahol <onemda@gmail.com> έγραψε: > Still wrong, You can decode images you linked just fine (albeit with > incorrect colors) with command: > > ffmpeg -subimage 1 -i IMAGE.dng ....rest of command. Oh ok, I thought you agreed to go with the -thumbnail option I proposed since it's more intuitive to the user. I suppose it won't be a problem if I suggest what argument to use.
On Sun, Mar 17, 2019 at 23:05:01 +0100, Paul B Mahol wrote: > Still wrong, You can decode images you linked just fine (albeit with > incorrect colors) with command: > > ffmpeg -subimage 1 -i IMAGE.dng ....rest of command. Shouldn't, ideally, these image files be demuxed as two image streams? Perhaps with the "main" image as the first stream. Just wondering, Moritz
On Mon, 18 Mar 2019 09:13:01 +0100 Moritz Barsnick <barsnick@gmx.net> wrote: > On Sun, Mar 17, 2019 at 23:05:01 +0100, Paul B Mahol wrote: > > Still wrong, You can decode images you linked just fine (albeit with > > incorrect colors) with command: > > > > ffmpeg -subimage 1 -i IMAGE.dng ....rest of command. > > Shouldn't, ideally, these image files be demuxed as two image streams? > Perhaps with the "main" image as the first stream. The DNG spec is pretty massive, and there's a huge amount of variations. There can easily be far more than two streams, there could be several "main" images and several previews in different sizes. Their order can vary too, it's not always the thumbnail first; thumbnails can also be omitted entirely. There's also several different encodings/compression types for the "main" images. I've used their libdng for a project. It's a big LGPL library implementing pretty much everything, but no distro really ships it, so it'd have to be embedded or built manually by the user. - Lauri
On Mon, Mar 18, 2019 at 09:13:01AM +0100, Moritz Barsnick wrote: > On Sun, Mar 17, 2019 at 23:05:01 +0100, Paul B Mahol wrote: > > Still wrong, You can decode images you linked just fine (albeit with > > incorrect colors) with command: > > > > ffmpeg -subimage 1 -i IMAGE.dng ....rest of command. > > Shouldn't, ideally, these image files be demuxed as two image streams? > Perhaps with the "main" image as the first stream. probably something like this, yes I think a good test for what "should" be done is to ask what "works" consider converting the data from one format to another, all data that source and destination supports should be preserved. A design that can only extract one image of many or one stream or one channel. Cannot preserve all information so it fails that simple use case. thx [...]
Yes, obviously this is not complete. As was said, the DNG spec is huge and I did bring up this concern in a personal email to Paul a few days ago. I was told that: > 3 months should be enough to finish most of specification, note you work on real world DNG files, so if some feature from spec is not present in any file you have less work to do which I absolutely agree with. The context of my contribution in this case is GSoC, so let's talk about that: Wouldn't it be better if by the end of GSoC, FFmpeg could open all or most of the DNG files that actually exist out there, instead of having only specific parts of the spec implemented thoroughly? >A design that can only extract one image of many or one stream or one channel. Cannot preserve all information so it fails that simple use case. > Shouldn't, ideally, these image files be demuxed as two image streams? Perhaps with the "main" image as the first stream. Ideally, yes of course. Realistically, I think the images with NewSubFileType != 0 and NewSubFileType != 4 are of secondary interest to decode, as they are commonly simply reduced resolution images of their counterparts. In any case, it will probably not be hard to add once the important parts are implemented. > Still wrong, There are DNG images without thumbnails. I suspected so but didn't have any examples to test with. I just found one. The following happens if -subimage 1 is used: [tiff @ 0x7fffe4099180] DNG images are not supported [tiff @ 0x7fffe4099180] Decoding embedded TIFF thumbnail in DNG image [tiff @ 0x7fffe4099180] This format is not supported (bpp=14, bppcount=1) Is this a problem? If so: I'm still not done reading the spec(s), but as far as I understand it, there is no way that a DNG image with NewSubFileType != 1 would be in a standard TIFF format that we can decode right now. Therefore, I propose a check for this in decode_frame (would also check if dng_mode is enabled) to prevent the above non-user friendly error from happening. I suspect NewSubFileType is not the right way to go though since the actual image format is not specified with it. I looked at the tags from some DNGs and I can't narrow it down to any other better field for this. Any better ideas? Should I perhaps just let it succeed or fail based on the existing decoding code, as it does above? The error checking in that code is the absolute truth of what is actually implemented after all. > I've used their libdng for a project. It's a big LGPL library implementing pretty much everything, but no distro really ships it, so it'd have to be embedded or built manually by the user. Definitely something to consider. Given the posted GSoC project idea, I assumed it was not possible/preferable for it to be embedded.
On Tue, Mar 19, 2019 at 03:03:20AM +0200, Nick Renieris wrote: > Yes, obviously this is not complete. As was said, the DNG spec is huge > and I did bring up this concern in a personal email to Paul a few days > ago. > I was told that: > > 3 months should be enough to finish most of specification, note you work on real world DNG files, so if some feature from spec is not present in any file you have less work to do > which I absolutely agree with. > The context of my contribution in this case is GSoC, so let's talk > about that: Wouldn't it be better if by the end of GSoC, FFmpeg could > open all or most of the DNG files that actually exist out there, > instead of having only specific parts of the spec implemented > thoroughly? Theres no need to implement every feature, but what FFmpeg supports should work. For example if we support audio and video demuxing from mpeg-ps and support audio and video muxing in mpeg-ts we should be able to convert one to the other with both audio and video. Similarly if we support demuxing "auxilary / secondary or what they are called images" and muxing then we should be able to connect these and not just be able to extract one image. Thats the ideal. The question how to implement this, or if this is the way to go or if this is too complex to do is up to the mentor for a GSoC project. It could be done by spliting into streams at the demuxer level, by using side data or decoding the images in sequence in the decoder or other ways. Each of these seem to have disadvanatges ... Thanks [...]
On 3/19/19, Nick Renieris <velocityra@gmail.com> wrote: > Yes, obviously this is not complete. As was said, the DNG spec is huge > and I did bring up this concern in a personal email to Paul a few days > ago. > I was told that: >> 3 months should be enough to finish most of specification, note you work >> on real world DNG files, so if some feature from spec is not present in >> any file you have less work to do > which I absolutely agree with. > The context of my contribution in this case is GSoC, so let's talk > about that: Wouldn't it be better if by the end of GSoC, FFmpeg could > open all or most of the DNG files that actually exist out there, > instead of having only specific parts of the spec implemented > thoroughly? > >>A design that can only extract one image of many or one stream or one >> channel. Cannot preserve all information so it fails that simple use case. > >> Shouldn't, ideally, these image files be demuxed as two image streams? >> Perhaps with the "main" image as the first stream. > > Ideally, yes of course. > Realistically, I think the images with NewSubFileType != 0 and > NewSubFileType != 4 are of secondary interest to decode, as they are > commonly simply reduced resolution images of their counterparts. > In any case, it will probably not be hard to add once the important > parts are implemented. > >> Still wrong, There are DNG images without thumbnails. > > I suspected so but didn't have any examples to test with. > I just found one. The following happens if -subimage 1 is used: > > [tiff @ 0x7fffe4099180] DNG images are not supported > [tiff @ 0x7fffe4099180] Decoding embedded TIFF thumbnail in DNG image > [tiff @ 0x7fffe4099180] This format is not supported (bpp=14, bppcount=1) > > Is this a problem? If so: > I'm still not done reading the spec(s), but as far as I understand it, > there is no way that a DNG image with NewSubFileType != 1 would be in > a standard TIFF format that we can decode right now. Therefore, I > propose a check for this in decode_frame (would also check if dng_mode > is enabled) to prevent the above non-user friendly error from > happening. > > I suspect NewSubFileType is not the right way to go though since the > actual image format is not specified with it. I looked at the tags > from some DNGs and I can't narrow it down to any other better field > for this. > > Any better ideas? Should I perhaps just let it succeed or fail based > on the existing decoding code, as it does above? The error checking in > that code is the absolute truth of what is actually implemented after > all. > >> I've used their libdng for a project. It's a big LGPL library implementing >> pretty much everything, but no distro really ships it, so it'd have to be >> embedded or built manually by the user. > > Definitely something to consider. Given the posted GSoC project idea, > I assumed it was not possible/preferable for it to be embedded. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Here is one file that works just fine with current ffmpeg: https://0x0.st/z8pf.dng
Hello, >Similarly if we support demuxing "auxilary / secondary or what they are called images" and muxing then we should be able to connect these and not just be able to extract one image. >Thats the ideal. The question how to implement this, or if this is the way to go or if this is too complex to do is up to the mentor for a GSoC project. >It could be done by spliting into streams at the demuxer level, by using side data or decoding the images in sequence in the decoder or other ways. Each of these seem to have disadvanatges ... Ok, but what does this have to do with my patch though? Isn't something like this possible with TIFF images too? As far as I know, that's not supported at the moment either, I only know of -subimage which I don't think does exactly what you mean. Not to mention, this patch is for preliminary support, I don't suppose you require a massive patchset that implements everything altogether? Besides, GSoC requirements state that I need to get a minor patch in, before I can even apply. This is what this patch is for. >Here is one file that works just fine with current ffmpeg: https://0x0.st/z8pf.dng Not sure why this was mentioned? It works with my patch too. I thought we came to an agreement, that by default it should show a message, instructing the user to show that they can decode the thumbnail with -subimage. This is what my patch does. Nick R. Στις Τρί, 19 Μαρ 2019 στις 12:58 μ.μ., ο/η Paul B Mahol <onemda@gmail.com> έγραψε: > > On 3/19/19, Nick Renieris <velocityra@gmail.com> wrote: > > Yes, obviously this is not complete. As was said, the DNG spec is huge > > and I did bring up this concern in a personal email to Paul a few days > > ago. > > I was told that: > >> 3 months should be enough to finish most of specification, note you work > >> on real world DNG files, so if some feature from spec is not present in > >> any file you have less work to do > > which I absolutely agree with. > > The context of my contribution in this case is GSoC, so let's talk > > about that: Wouldn't it be better if by the end of GSoC, FFmpeg could > > open all or most of the DNG files that actually exist out there, > > instead of having only specific parts of the spec implemented > > thoroughly? > > > >>A design that can only extract one image of many or one stream or one > >> channel. Cannot preserve all information so it fails that simple use case. > > > >> Shouldn't, ideally, these image files be demuxed as two image streams? > >> Perhaps with the "main" image as the first stream. > > > > Ideally, yes of course. > > Realistically, I think the images with NewSubFileType != 0 and > > NewSubFileType != 4 are of secondary interest to decode, as they are > > commonly simply reduced resolution images of their counterparts. > > In any case, it will probably not be hard to add once the important > > parts are implemented. > > > >> Still wrong, There are DNG images without thumbnails. > > > > I suspected so but didn't have any examples to test with. > > I just found one. The following happens if -subimage 1 is used: > > > > [tiff @ 0x7fffe4099180] DNG images are not supported > > [tiff @ 0x7fffe4099180] Decoding embedded TIFF thumbnail in DNG image > > [tiff @ 0x7fffe4099180] This format is not supported (bpp=14, bppcount=1) > > > > Is this a problem? If so: > > I'm still not done reading the spec(s), but as far as I understand it, > > there is no way that a DNG image with NewSubFileType != 1 would be in > > a standard TIFF format that we can decode right now. Therefore, I > > propose a check for this in decode_frame (would also check if dng_mode > > is enabled) to prevent the above non-user friendly error from > > happening. > > > > I suspect NewSubFileType is not the right way to go though since the > > actual image format is not specified with it. I looked at the tags > > from some DNGs and I can't narrow it down to any other better field > > for this. > > > > Any better ideas? Should I perhaps just let it succeed or fail based > > on the existing decoding code, as it does above? The error checking in > > that code is the absolute truth of what is actually implemented after > > all. > > > >> I've used their libdng for a project. It's a big LGPL library implementing > >> pretty much everything, but no distro really ships it, so it'd have to be > >> embedded or built manually by the user. > > > > Definitely something to consider. Given the posted GSoC project idea, > > I assumed it was not possible/preferable for it to be embedded. > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > Here is one file that works just fine with current ffmpeg: > https://0x0.st/z8pf.dng > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 3/20/19, Nick Renieris <velocityra@gmail.com> wrote: > Hello, > >>Similarly if we support demuxing "auxilary / secondary or what they are >> called images" and muxing then we should be able to connect these and not >> just be able to extract one image. >>Thats the ideal. The question how to implement this, or if this is the way >> to go or if this is too complex to do is up to the mentor for a GSoC >> project. >>It could be done by spliting into streams at the demuxer level, by using >> side data or decoding the images in sequence in the decoder or other ways. >> Each of these seem to have disadvanatges ... > > Ok, but what does this have to do with my patch though? Isn't > something like this possible with TIFF images too? As far as I know, > that's not supported at the moment either, I only know of -subimage > which I don't think does exactly what you mean. > > Not to mention, this patch is for preliminary support, I don't suppose > you require a massive patchset that implements everything altogether? > Besides, GSoC requirements state that I need to get a minor patch in, > before I can even apply. This is what this patch is for. > >>Here is one file that works just fine with current ffmpeg: >> https://0x0.st/z8pf.dng > > Not sure why this was mentioned? It works with my patch too. > I thought we came to an agreement, that by default it should show a > message, instructing the user to show that they can decode the > thumbnail with -subimage. > This is what my patch does. > > Nick R. > > Στις Τρί, 19 Μαρ 2019 στις 12:58 μ.μ., ο/η Paul B Mahol > <onemda@gmail.com> έγραψε: >> >> On 3/19/19, Nick Renieris <velocityra@gmail.com> wrote: >> > Yes, obviously this is not complete. As was said, the DNG spec is huge >> > and I did bring up this concern in a personal email to Paul a few days >> > ago. >> > I was told that: >> >> 3 months should be enough to finish most of specification, note you >> >> work >> >> on real world DNG files, so if some feature from spec is not present in >> >> any file you have less work to do >> > which I absolutely agree with. >> > The context of my contribution in this case is GSoC, so let's talk >> > about that: Wouldn't it be better if by the end of GSoC, FFmpeg could >> > open all or most of the DNG files that actually exist out there, >> > instead of having only specific parts of the spec implemented >> > thoroughly? >> > >> >>A design that can only extract one image of many or one stream or one >> >> channel. Cannot preserve all information so it fails that simple use >> >> case. >> > >> >> Shouldn't, ideally, these image files be demuxed as two image streams? >> >> Perhaps with the "main" image as the first stream. >> > >> > Ideally, yes of course. >> > Realistically, I think the images with NewSubFileType != 0 and >> > NewSubFileType != 4 are of secondary interest to decode, as they are >> > commonly simply reduced resolution images of their counterparts. >> > In any case, it will probably not be hard to add once the important >> > parts are implemented. >> > >> >> Still wrong, There are DNG images without thumbnails. >> > >> > I suspected so but didn't have any examples to test with. >> > I just found one. The following happens if -subimage 1 is used: >> > >> > [tiff @ 0x7fffe4099180] DNG images are not supported >> > [tiff @ 0x7fffe4099180] Decoding embedded TIFF thumbnail in DNG image >> > [tiff @ 0x7fffe4099180] This format is not supported (bpp=14, >> > bppcount=1) >> > >> > Is this a problem? If so: >> > I'm still not done reading the spec(s), but as far as I understand it, >> > there is no way that a DNG image with NewSubFileType != 1 would be in >> > a standard TIFF format that we can decode right now. Therefore, I >> > propose a check for this in decode_frame (would also check if dng_mode >> > is enabled) to prevent the above non-user friendly error from >> > happening. >> > >> > I suspect NewSubFileType is not the right way to go though since the >> > actual image format is not specified with it. I looked at the tags >> > from some DNGs and I can't narrow it down to any other better field >> > for this. >> > >> > Any better ideas? Should I perhaps just let it succeed or fail based >> > on the existing decoding code, as it does above? The error checking in >> > that code is the absolute truth of what is actually implemented after >> > all. DNG I posted should be decodeable by default, without need for extra option(s). The subimage option is intended to decode Nth image as is stored in file. The thumbnail option should be implemented to decode only first/best thumbnail. There is way to detect them and ignore them when not that option is set. >> > >> >> I've used their libdng for a project. It's a big LGPL library >> >> implementing >> >> pretty much everything, but no distro really ships it, so it'd have to >> >> be >> >> embedded or built manually by the user. >> > >> > Definitely something to consider. Given the posted GSoC project idea, >> > I assumed it was not possible/preferable for it to be embedded. >> > _______________________________________________ >> > ffmpeg-devel mailing list >> > ffmpeg-devel@ffmpeg.org >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > >> >> Here is one file that works just fine with current ffmpeg: >> https://0x0.st/z8pf.dng >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Στις Τετ, 20 Μαρ 2019 στις 4:17 μ.μ., ο/η Paul B Mahol <onemda@gmail.com> έγραψε: > DNG I posted should be decodeable by default, without need for extra option(s). The DNG contents themselves? As in, not just the thumbnail? That's what the whole GSoC proposal is about though... What about: >On Tue, Mar 19, 2019 at 03:03:20AM +0200, Nick Renieris wrote: >Not to mention, this patch is for preliminary support, I don't suppose you require a massive patchset that implements everything altogether? Besides, GSoC requirements state that I need to get a minor patch in, before I can even apply. This is what this patch is for. > The subimage option is intended to decode Nth image as is stored in file. Nth image? The option is a boolean. _If_ it was an integer though: Given what you said, for -thumbnail I would need to check for NewSubfileType's Bit0==1 and for -subimage, Bit1==1? (Here's where I'm getting my info from so that we're on the same page: https://www.awaresystems.be/imaging/tiff/tifftags/newsubfiletype.html) But it's not an integer as far as I can see, so all of the above is out of scope for this patch. > The thumbnail option should be implemented to decode only first/best thumbnail. Sounds easy enough. > There is way to detect them and ignore them when not that option is set. Detect/ignore the thumbnails? Yes, that's easy.
Any consensus on using libdng? Στις Τετ, 20 Μαρ 2019 στις 5:03 μ.μ., ο/η Nick Renieris <velocityra@gmail.com> έγραψε: > > Στις Τετ, 20 Μαρ 2019 στις 4:17 μ.μ., ο/η Paul B Mahol > <onemda@gmail.com> έγραψε: > > DNG I posted should be decodeable by default, without need for extra option(s). > > The DNG contents themselves? As in, not just the thumbnail? That's > what the whole GSoC proposal is about though... What about: > >On Tue, Mar 19, 2019 at 03:03:20AM +0200, Nick Renieris wrote: > >Not to mention, this patch is for preliminary support, I don't suppose you require a massive patchset that implements everything altogether? Besides, GSoC requirements state that I need to get a minor patch in, before I can even apply. This is what this patch is for. > > > The subimage option is intended to decode Nth image as is stored in file. > > Nth image? The option is a boolean. > > _If_ it was an integer though: > > Given what you said, for -thumbnail I would need to check for > NewSubfileType's Bit0==1 and for -subimage, Bit1==1? > (Here's where I'm getting my info from so that we're on the same page: > https://www.awaresystems.be/imaging/tiff/tifftags/newsubfiletype.html) > > But it's not an integer as far as I can see, so all of the above is > out of scope for this patch. > > > The thumbnail option should be implemented to decode only first/best thumbnail. > > Sounds easy enough. > > > There is way to detect them and ignore them when not that option is set. > > Detect/ignore the thumbnails? Yes, that's easy.
I don't mind either way, but I'm writing my GSoC proposal at the moment and wanted an answer. If there's no clear answer, is it something TBD that I don't need to specify in the proposal?
On 4/2/19, Nick Renieris <velocityra@gmail.com> wrote: > I don't mind either way, but I'm writing my GSoC proposal at the > moment and wanted an answer. > > If there's no clear answer, is it something TBD that I don't need to > specify in the proposal? I do not think we can use libdng.
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c index 112f5b52f4..341c280736 100644 --- a/libavcodec/tiff.c +++ b/libavcodec/tiff.c @@ -54,7 +54,9 @@ typedef struct TiffContext { AVCodecContext *avctx; GetByteContext gb; + /** options */ int get_subimage; + int get_dng_thumbnail; int width, height; unsigned int bpp, bppcount; @@ -70,6 +72,7 @@ typedef struct TiffContext { int fill_order; uint32_t res[4]; + int dng_mode; /** denotes that this is a DNG image */ int is_bayer; uint8_t pattern[4]; unsigned white_level; @@ -1077,7 +1080,7 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) case TIFF_SUB_IFDS: s->sub_ifd = value; break; - case TIFF_WHITE_LEVEL: + case DNG_WHITE_LEVEL: s->white_level = value; break; case TIFF_CFA_PATTERN_DIM: @@ -1322,6 +1325,20 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) case TIFF_SOFTWARE_NAME: ADD_METADATA(count, "software", NULL); break; + case DNG_VERSION: + if (count == 4) { + unsigned int ver[4]; + ver[0] = ff_tget(&s->gb, type, s->le); + ver[1] = ff_tget(&s->gb, type, s->le); + ver[2] = ff_tget(&s->gb, type, s->le); + ver[3] = ff_tget(&s->gb, type, s->le); + + av_log(s->avctx, AV_LOG_DEBUG, "DNG file, version %u.%u.%u.%u\n", + ver[0], ver[1], ver[2], ver[3]); + + s->dng_mode = 1; + } + break; default: if (s->avctx->err_recognition & AV_EF_EXPLODE) { av_log(s->avctx, AV_LOG_ERROR, @@ -1375,6 +1392,7 @@ again: s->fill_order = 0; s->white_level = 0; s->is_bayer = 0; + s->dng_mode = 0; free_geotags(s); // Reset these offsets so we can tell if they were set this frame @@ -1389,6 +1407,14 @@ again: return ret; } + if (s->dng_mode) { + av_log(avctx, AV_LOG_ERROR, "DNG images are not supported\n"); + av_log(avctx, AV_LOG_INFO, "Consider using -dng_thumbnail for decoding the embedded thumbail instead\n"); + + if (!s->get_dng_thumbnail) + return AVERROR_PATCHWELCOME; + } + if (s->sub_ifd && s->get_subimage) { off = s->sub_ifd; if (off >= UINT_MAX - 14 || avpkt->size < off + 14) { @@ -1607,6 +1633,7 @@ static av_cold int tiff_end(AVCodecContext *avctx) #define OFFSET(x) offsetof(TiffContext, x) static const AVOption tiff_options[] = { { "subimage", "decode subimage instead if available", OFFSET(get_subimage), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_VIDEO_PARAM }, + { "dng_thumbnail", "decode embedded thumbnail for DNG images, if available", OFFSET(get_dng_thumbnail), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_VIDEO_PARAM }, { NULL }, }; diff --git a/libavcodec/tiff.h b/libavcodec/tiff.h index 4b08650108..3cede299f6 100644 --- a/libavcodec/tiff.h +++ b/libavcodec/tiff.h @@ -20,7 +20,7 @@ /** * @file - * TIFF tables + * TIFF constants & data structures * * For more information about the TIFF format, check the official docs at: * http://partners.adobe.com/public/developer/tiff/index.html @@ -33,7 +33,7 @@ #include <stdint.h> #include "tiff_common.h" -/** abridged list of TIFF tags */ +/** abridged list of TIFF and TIFF/EP tags */ enum TiffTags { TIFF_SUBFILE = 0xfe, TIFF_WIDTH = 0x100, @@ -85,10 +85,17 @@ enum TiffTags { TIFF_GEO_KEY_DIRECTORY = 0x87AF, TIFF_GEO_DOUBLE_PARAMS = 0x87B0, TIFF_GEO_ASCII_PARAMS = 0x87B1, - TIFF_WHITE_LEVEL = 0xC61D, }; -/** list of TIFF compression types */ +/** abridged list of DNG tags */ +enum DngTags +{ + DNG_VERSION = 0xC612, + DNG_BACKWARD_VERSION = 0xC613, + DNG_WHITE_LEVEL = 0xC61D, +}; + +/** list of TIFF, TIFF/EP and DNG compression types */ enum TiffCompr { TIFF_RAW = 1, TIFF_CCITT_RLE, @@ -151,6 +158,7 @@ enum TiffGeoTagKey { TIFF_VERTICAL_UNITS_GEOKEY = 4099 }; +/** list of TIFF, TIFF/AP and DNG PhotometricInterpretation (TIFF_PHOTOMETRIC) values */ enum TiffPhotometric { TIFF_PHOTOMETRIC_NONE = -1, TIFF_PHOTOMETRIC_WHITE_IS_ZERO, /* mono or grayscale, 0 is white */ @@ -163,7 +171,7 @@ enum TiffPhotometric { TIFF_PHOTOMETRIC_CIE_LAB = 8, /* 1976 CIE L*a*b* */ TIFF_PHOTOMETRIC_ICC_LAB, /* ICC L*a*b* */ TIFF_PHOTOMETRIC_ITU_LAB, /* ITU L*a*b* */ - TIFF_PHOTOMETRIC_CFA = 32803, /* Color Filter Array (DNG) */ + TIFF_PHOTOMETRIC_CFA = 32803, /* Color Filter Array (TIFF/AP and DNG) */ TIFF_PHOTOMETRIC_LOG_L = 32844, /* CIE Log2(L) */ TIFF_PHOTOMETRIC_LOG_LUV, /* CIE Log L*u*v* */ TIFF_PHOTOMETRIC_LINEAR_RAW = 34892, /* Linear Raw (DNG) */ diff --git a/libavformat/img2.c b/libavformat/img2.c index 8432cc0955..16bc9d2abd 100644 --- a/libavformat/img2.c +++ b/libavformat/img2.c @@ -51,6 +51,7 @@ const IdStrMap ff_img_tags[] = { { AV_CODEC_ID_TARGA, "tga" }, { AV_CODEC_ID_TIFF, "tiff" }, { AV_CODEC_ID_TIFF, "tif" }, + { AV_CODEC_ID_TIFF, "dng" }, { AV_CODEC_ID_SGI, "sgi" }, { AV_CODEC_ID_PTX, "ptx" }, { AV_CODEC_ID_PCX, "pcx" },