diff mbox series

[FFmpeg-devel,v3,1/2] lavu/dovi_meta - add fields for ext_mapping_idc

Message ID 0101018fa0ff6506-a7840850-87e7-4b51-9ed3-a926167d34e0-000000@us-west-2.amazonses.com
State New
Headers show
Series [FFmpeg-devel,v3,1/2] lavu/dovi_meta - add fields for ext_mapping_idc | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Cosmin Stejerean May 22, 2024, 3:50 p.m. UTC
From: Cosmin Stejerean <cosmin@cosmin.at>

---
 libavutil/dovi_meta.h | 2 ++
 libavutil/version.h   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Niklas Haas May 23, 2024, 11:03 a.m. UTC | #1
On Wed, 22 May 2024 15:50:43 +0000 Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
> From: Cosmin Stejerean <cosmin@cosmin.at>
> 
> ---
>  libavutil/dovi_meta.h | 2 ++
>  libavutil/version.h   | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> index e10332f8d7..e168075a24 100644
> --- a/libavutil/dovi_meta.h
> +++ b/libavutil/dovi_meta.h
> @@ -91,6 +91,8 @@ typedef struct AVDOVIRpuDataHeader {
>      uint8_t spatial_resampling_filter_flag;
>      uint8_t el_spatial_resampling_filter_flag;
>      uint8_t disable_residual_flag;
> +    uint8_t ext_mapping_idc_0_4; /* extended base layer inverse mapping indicator */
> +    uint8_t ext_mapping_idc_5_7; /* reserved */
>  } AVDOVIRpuDataHeader;

What value ranges have you seen for this indicator? Is it possible that
some values would extend the RPU in other ways, adding more bits that we
need to parse?

Maybe we should enforce this to be a well-known value just to be on the
safe side, until we receive public documentation on its purpose.

>  
>  enum AVDOVIMappingMethod {
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 3221c4c592..9c7146c228 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  59
> -#define LIBAVUTIL_VERSION_MINOR  19
> +#define LIBAVUTIL_VERSION_MINOR  20
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> -- 
> 2.42.1
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Cosmin Stejerean May 23, 2024, 10:17 p.m. UTC | #2
> On May 23, 2024, at 4:03 AM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
> 
> On Wed, 22 May 2024 15:50:43 +0000 Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>> From: Cosmin Stejerean <cosmin@cosmin.at>
>> 
>> ---
>> libavutil/dovi_meta.h | 2 ++
>> libavutil/version.h   | 2 +-
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
>> index e10332f8d7..e168075a24 100644
>> --- a/libavutil/dovi_meta.h
>> +++ b/libavutil/dovi_meta.h
>> @@ -91,6 +91,8 @@ typedef struct AVDOVIRpuDataHeader {
>>     uint8_t spatial_resampling_filter_flag;
>>     uint8_t el_spatial_resampling_filter_flag;
>>     uint8_t disable_residual_flag;
>> +    uint8_t ext_mapping_idc_0_4; /* extended base layer inverse mapping indicator */
>> +    uint8_t ext_mapping_idc_5_7; /* reserved */
>> } AVDOVIRpuDataHeader;
> 
> What value ranges have you seen for this indicator? Is it possible that
> some values would extend the RPU in other ways, adding more bits that we
> need to parse?

So far I've only seen two sets of values. 

The samples from the iPhone:

ext_mapping_idc[4:0] = 0
ext_mapping_idc[7:5] = 0

The samples from Xiaomi and Oppo:

ext_mapping_idc[4:0] = 1
ext_mapping_idc[7:5] = 4

I don't have any additional information on what the possible range of values could be so far.
As far as I can tell it has no impact on the size of the subsequent fields, at least for what I've encountered so far.


> 
> Maybe we should enforce this to be a well-known value just to be on the
> safe side, until we receive public documentation on its purpose.
> 

I think to avoid being brittle we should allow the entire range of values for now until we know better, and just ensure it roundtrips correctly (which I've verified by running the validator on the 10.4 output transcoded from 8.4 input).

Once we track down more info we can try to add more validation on top of it.

- Cosmin
diff mbox series

Patch

diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
index e10332f8d7..e168075a24 100644
--- a/libavutil/dovi_meta.h
+++ b/libavutil/dovi_meta.h
@@ -91,6 +91,8 @@  typedef struct AVDOVIRpuDataHeader {
     uint8_t spatial_resampling_filter_flag;
     uint8_t el_spatial_resampling_filter_flag;
     uint8_t disable_residual_flag;
+    uint8_t ext_mapping_idc_0_4; /* extended base layer inverse mapping indicator */
+    uint8_t ext_mapping_idc_5_7; /* reserved */
 } AVDOVIRpuDataHeader;
 
 enum AVDOVIMappingMethod {
diff --git a/libavutil/version.h b/libavutil/version.h
index 3221c4c592..9c7146c228 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  59
-#define LIBAVUTIL_VERSION_MINOR  19
+#define LIBAVUTIL_VERSION_MINOR  20
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \