[FFmpeg-devel] avcodec/tiff: Add support for recognizing DNG files

Submitted by velocityra@gmail.com on March 17, 2019, 9:29 p.m.

Details

Message ID 20190317212934.18060-1-velocityra@gmail.com
State New
Headers show

Commit Message

velocityra@gmail.com March 17, 2019, 9:29 p.m.
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(-)

Comments

Paul B Mahol March 17, 2019, 10:05 p.m.
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.
velocityra@gmail.com March 18, 2019, 12:47 a.m.
Στις Δευ, 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.
Moritz Barsnick March 18, 2019, 8:13 a.m.
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
Lauri Kasanen March 18, 2019, 8:32 a.m.
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
Michael Niedermayer March 18, 2019, 9:19 a.m.
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

[...]
velocityra@gmail.com March 19, 2019, 1:03 a.m.
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.
Michael Niedermayer March 19, 2019, 10:39 a.m.
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

[...]
Paul B Mahol March 19, 2019, 10:58 a.m.
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
velocityra@gmail.com March 20, 2019, 4:10 a.m.
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
Paul B Mahol March 20, 2019, 2:08 p.m.
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
>
velocityra@gmail.com March 20, 2019, 3:03 p.m.
Στις Τετ, 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.
velocityra@gmail.com April 2, 2019, 12:27 p.m.
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.
velocityra@gmail.com April 2, 2019, 12:32 p.m.
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?
Paul B Mahol April 2, 2019, 1:50 p.m.
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.

Patch hide | download patch | download mbox

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"      },