diff mbox series

[FFmpeg-devel,1/4] avformat/avformat: add a new disposition to signal the stream is an HDR gainmap

Message ID 20240925225219.3060-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/4] avformat/avformat: add a new disposition to signal the stream is an HDR gainmap | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer Sept. 25, 2024, 10:52 p.m. UTC
HDR images photos taken by certain cameras split this as a separate image.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/avformat.h | 5 +++++
 libavformat/dump.c     | 2 ++
 2 files changed, 7 insertions(+)

Comments

Anton Khirnov Sept. 26, 2024, 2:17 p.m. UTC | #1
Quoting James Almer (2024-09-26 14:06:25)
> On 9/26/2024 3:00 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-09-26 00:52:16)
> >> HDR images photos taken by certain cameras split this as a separate image.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavformat/avformat.h | 5 +++++
> >>   libavformat/dump.c     | 2 ++
> >>   2 files changed, 7 insertions(+)
> >>
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 56c1c80289..6d9f5c4399 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -718,6 +718,11 @@ typedef struct AVIndexEntry {
> >>    * Annex G/H, or HEVC Annex F).
> >>    */
> >>   #define AV_DISPOSITION_MULTILAYER           (1 << 21)
> >> +/**
> >> + * The video stream contains an HDR gainmap. Only ever used with
> >> + * AV_DISPOSITION_DEPENDENT.
> >> + */
> >> +#define AV_DISPOSITION_GAINMAP              (1 << 22)
> > 
> > Presumably we want this information available in codecs and filters as
> > well, so then should it not be side data instead?
> 
> There is no other information than "This is a gainmap", and that's 
> container level information (Same as "This is a tile" for heif). How 
> would side data work for this?

You could e.g. have side data that has no payload, and its presence
signals the same information.

> I'm including it in the tile grid stream group in patch 4/4, so that 
> should give the caller all the information they need.

Assuming all they ever need is handle everything manually.
Anton Khirnov Sept. 26, 2024, 2:35 p.m. UTC | #2
Quoting James Almer (2024-09-26 16:25:11)
> On 9/26/2024 11:17 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-09-26 14:06:25)
> >> On 9/26/2024 3:00 AM, Anton Khirnov wrote:
> >>> Quoting James Almer (2024-09-26 00:52:16)
> >>>> HDR images photos taken by certain cameras split this as a separate image.
> >>>>
> >>>> Signed-off-by: James Almer <jamrial@gmail.com>
> >>>> ---
> >>>>    libavformat/avformat.h | 5 +++++
> >>>>    libavformat/dump.c     | 2 ++
> >>>>    2 files changed, 7 insertions(+)
> >>>>
> >>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >>>> index 56c1c80289..6d9f5c4399 100644
> >>>> --- a/libavformat/avformat.h
> >>>> +++ b/libavformat/avformat.h
> >>>> @@ -718,6 +718,11 @@ typedef struct AVIndexEntry {
> >>>>     * Annex G/H, or HEVC Annex F).
> >>>>     */
> >>>>    #define AV_DISPOSITION_MULTILAYER           (1 << 21)
> >>>> +/**
> >>>> + * The video stream contains an HDR gainmap. Only ever used with
> >>>> + * AV_DISPOSITION_DEPENDENT.
> >>>> + */
> >>>> +#define AV_DISPOSITION_GAINMAP              (1 << 22)
> >>>
> >>> Presumably we want this information available in codecs and filters as
> >>> well, so then should it not be side data instead?
> >>
> >> There is no other information than "This is a gainmap", and that's
> >> container level information (Same as "This is a tile" for heif). How
> >> would side data work for this?
> > 
> > You could e.g. have side data that has no payload, and its presence
> > signals the same information.
> 
> Is there precedent for this?

Not as far as I know. But I also don't see why couldn't it be done.

> Would it be a side data with size 0 (but data still pointing to an
> AVBufferRef)?

Packet side data is not refcounted. Otherwise yes, I imagine it would
have 0 size, unless we figure out some useful content.

> > 
> >> I'm including it in the tile grid stream group in patch 4/4, so that
> >> should give the caller all the information they need.
> > 
> > Assuming all they ever need is handle everything manually.
> 
> I don't understand what you mean.

What I mean is - if we ever want to propagate this information to
decoders/filters, we probably want it to be side data. Then the
disposition would be redudundant. And since we are not far from running
out of disposition space, we might as well make it side data only.

It could also conceivably be a frame/packet flag.
diff mbox series

Patch

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 56c1c80289..6d9f5c4399 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -718,6 +718,11 @@  typedef struct AVIndexEntry {
  * Annex G/H, or HEVC Annex F).
  */
 #define AV_DISPOSITION_MULTILAYER           (1 << 21)
+/**
+ * The video stream contains an HDR gainmap. Only ever used with
+ * AV_DISPOSITION_DEPENDENT.
+ */
+#define AV_DISPOSITION_GAINMAP              (1 << 22)
 
 /**
  * @return The AV_DISPOSITION_* flag corresponding to disp or a negative error
diff --git a/libavformat/dump.c b/libavformat/dump.c
index f20c2c4953..5178f19685 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -589,6 +589,8 @@  static void dump_disposition(int disposition, int log_level)
         av_log(NULL, log_level, " (non-diegetic)");
     if (disposition & AV_DISPOSITION_MULTILAYER)
         av_log(NULL, log_level, " (multilayer)");
+    if (disposition & AV_DISPOSITION_GAINMAP)
+        av_log(NULL, log_level, " (hdr gainmap)");
 }
 
 /* "user interface" functions */