Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avformat/apvdec: export color information
@ 2025-04-27 17:53 James Almer
  2025-04-27 18:29 ` Mark Thompson
  2025-04-27 18:57 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt
  0 siblings, 2 replies; 8+ messages in thread
From: James Almer @ 2025-04-27 17:53 UTC (permalink / raw)
  To: ffmpeg-devel

apv_read_header() reads enough information that the generic demux code doesn't
attempt to read a frame to fill missing fields in codecpar, so make sure it's
set here.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/apvdec.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
index e1ac34b003..94d2ce9965 100644
--- a/libavformat/apvdec.c
+++ b/libavformat/apvdec.c
@@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
     uint8_t  chroma_format_idc;
     uint8_t  bit_depth_minus8;
 
+    int      color_primaries;
+    int      transfer_characteristics;
+    int      matrix_coefficients;
+    int      full_range_flag;
+
     enum AVPixelFormat pixel_format;
 } APVHeaderInfo;
 
@@ -55,6 +60,8 @@ static int apv_extract_header_info(APVHeaderInfo *info,
 {
     int zero, byte, bit_depth_index;
 
+    memset(info, 0, sizeof(*info));
+
     info->pbu_type = bytestream2_get_byte(gbc);
     info->group_id = bytestream2_get_be16(gbc);
 
@@ -107,10 +114,24 @@ static int apv_extract_header_info(APVHeaderInfo *info,
     // Ignore capture_time_distance.
     bytestream2_skip(gbc, 1);
 
-    zero = bytestream2_get_byte(gbc);
-    if (zero != 0)
+    // two reserved_zero_8bits
+    if (bytestream2_get_be16(gbc) != 0)
         return AVERROR_INVALIDDATA;
 
+    // color_description_present_flag
+    if (bytestream2_peek_byte(gbc) >> 7) {
+        // We can read 32 bits as tile info is guaranteed to be present after this.
+        unsigned color_description = bytestream2_get_be32(gbc);
+
+        if (bytestream2_get_bytes_left(gbc) <= 0)
+            return AVERROR_INVALIDDATA;
+
+        info->color_primaries          = (color_description >> 23) & 0xff;
+        info->transfer_characteristics = (color_description >> 15) & 0xff;
+        info->matrix_coefficients      = (color_description >>  7) & 0xff;
+        info->full_range_flag          = (color_description >>  6) & 1;
+    }
+
     return 1;
 }
 
@@ -157,7 +178,7 @@ static int apv_read_header(AVFormatContext *s)
     AVStream *st;
     GetByteContext gbc;
     APVHeaderInfo header;
-    uint8_t buffer[28];
+    uint8_t buffer[64];
     uint32_t au_size, signature, pbu_size;
     int err, size;
 
@@ -201,6 +222,11 @@ static int apv_read_header(AVFormatContext *s)
     st->codecpar->level      = header.level_idc;
     st->codecpar->width      = header.frame_width;
     st->codecpar->height     = header.frame_height;
+    st->codecpar->color_primaries = header.color_primaries;
+    st->codecpar->color_trc       = header.transfer_characteristics;
+    st->codecpar->color_space     = header.matrix_coefficients;
+    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
+                                                           : AVCOL_RANGE_MPEG;
 
     st->avg_frame_rate = (AVRational){ 30, 1 };
     avpriv_set_pts_info(st, 64, 1, 30);
-- 
2.49.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/apvdec: export color information
  2025-04-27 17:53 [FFmpeg-devel] [PATCH] avformat/apvdec: export color information James Almer
@ 2025-04-27 18:29 ` Mark Thompson
  2025-04-27 19:22   ` James Almer
                     ` (2 more replies)
  2025-04-27 18:57 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt
  1 sibling, 3 replies; 8+ messages in thread
From: Mark Thompson @ 2025-04-27 18:29 UTC (permalink / raw)
  To: ffmpeg-devel

On 27/04/2025 18:53, James Almer wrote:
> apv_read_header() reads enough information that the generic demux code doesn't
> attempt to read a frame to fill missing fields in codecpar, so make sure it's
> set here.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/apvdec.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
> index e1ac34b003..94d2ce9965 100644
> --- a/libavformat/apvdec.c
> +++ b/libavformat/apvdec.c
> @@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
>      uint8_t  chroma_format_idc;
>      uint8_t  bit_depth_minus8;
>  
> +    int      color_primaries;
> +    int      transfer_characteristics;
> +    int      matrix_coefficients;
> +    int      full_range_flag;
> +
>      enum AVPixelFormat pixel_format;
>  } APVHeaderInfo;
>  
> @@ -55,6 +60,8 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>  {
>      int zero, byte, bit_depth_index;
>  
> +    memset(info, 0, sizeof(*info));
> +
>      info->pbu_type = bytestream2_get_byte(gbc);
>      info->group_id = bytestream2_get_be16(gbc);
>  
> @@ -107,10 +114,24 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>      // Ignore capture_time_distance.
>      bytestream2_skip(gbc, 1);
>  
> -    zero = bytestream2_get_byte(gbc);
> -    if (zero != 0)
> +    // two reserved_zero_8bits
> +    if (bytestream2_get_be16(gbc) != 0)
>          return AVERROR_INVALIDDATA;
>  
> +    // color_description_present_flag
> +    if (bytestream2_peek_byte(gbc) >> 7) {
> +        // We can read 32 bits as tile info is guaranteed to be present after this.
> +        unsigned color_description = bytestream2_get_be32(gbc);
> +
> +        if (bytestream2_get_bytes_left(gbc) <= 0)
> +            return AVERROR_INVALIDDATA;
> +
> +        info->color_primaries          = (color_description >> 23) & 0xff;
> +        info->transfer_characteristics = (color_description >> 15) & 0xff;
> +        info->matrix_coefficients      = (color_description >>  7) & 0xff;

For these "unspecified" has value 2, which will need to be set somewhere in no-description case rather than zero-init.  (Matrix coefficients 0 means identity, i.e. RGB, which we definitely don't want.)

> +        info->full_range_flag          = (color_description >>  6) & 1;
> +    }
> +
>      return 1;
>  }
>  
> @@ -157,7 +178,7 @@ static int apv_read_header(AVFormatContext *s)
>      AVStream *st;
>      GetByteContext gbc;
>      APVHeaderInfo header;
> -    uint8_t buffer[28];
> +    uint8_t buffer[64];

Could be smaller, but doesn't really matter.

>      uint32_t au_size, signature, pbu_size;
>      int err, size;
>  
> @@ -201,6 +222,11 @@ static int apv_read_header(AVFormatContext *s)
>      st->codecpar->level      = header.level_idc;
>      st->codecpar->width      = header.frame_width;
>      st->codecpar->height     = header.frame_height;
> +    st->codecpar->color_primaries = header.color_primaries;
> +    st->codecpar->color_trc       = header.transfer_characteristics;
> +    st->codecpar->color_space     = header.matrix_coefficients;
> +    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
> +                                                           : AVCOL_RANGE_MPEG;

Suggest st->codecpar->chroma_location = AVCHROMA_LOC_TOPLEFT as well?

>  
>      st->avg_frame_rate = (AVRational){ 30, 1 };
>      avpriv_set_pts_info(st, 64, 1, 30);

Thanks,

- Mark

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/apvdec: export color information
  2025-04-27 17:53 [FFmpeg-devel] [PATCH] avformat/apvdec: export color information James Almer
  2025-04-27 18:29 ` Mark Thompson
@ 2025-04-27 18:57 ` Andreas Rheinhardt
  1 sibling, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2025-04-27 18:57 UTC (permalink / raw)
  To: ffmpeg-devel

James Almer:
> apv_read_header() reads enough information that the generic demux code doesn't
> attempt to read a frame to fill missing fields in codecpar, so make sure it's
> set here.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/apvdec.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
> index e1ac34b003..94d2ce9965 100644
> --- a/libavformat/apvdec.c
> +++ b/libavformat/apvdec.c
> @@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
>      uint8_t  chroma_format_idc;
>      uint8_t  bit_depth_minus8;
>  
> +    int      color_primaries;
> +    int      transfer_characteristics;
> +    int      matrix_coefficients;
> +    int      full_range_flag;
> +
>      enum AVPixelFormat pixel_format;
>  } APVHeaderInfo;
>  
> @@ -55,6 +60,8 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>  {
>      int zero, byte, bit_depth_index;
>  
> +    memset(info, 0, sizeof(*info));
> +
>      info->pbu_type = bytestream2_get_byte(gbc);
>      info->group_id = bytestream2_get_be16(gbc);
>  
> @@ -107,10 +114,24 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>      // Ignore capture_time_distance.
>      bytestream2_skip(gbc, 1);
>  
> -    zero = bytestream2_get_byte(gbc);
> -    if (zero != 0)
> +    // two reserved_zero_8bits
> +    if (bytestream2_get_be16(gbc) != 0)
>          return AVERROR_INVALIDDATA;
>  
> +    // color_description_present_flag
> +    if (bytestream2_peek_byte(gbc) >> 7) {
> +        // We can read 32 bits as tile info is guaranteed to be present after this.
> +        unsigned color_description = bytestream2_get_be32(gbc);
> +
> +        if (bytestream2_get_bytes_left(gbc) <= 0)
> +            return AVERROR_INVALIDDATA;

You should check whether enough data is present before reading.

> +
> +        info->color_primaries          = (color_description >> 23) & 0xff;
> +        info->transfer_characteristics = (color_description >> 15) & 0xff;
> +        info->matrix_coefficients      = (color_description >>  7) & 0xff;
> +        info->full_range_flag          = (color_description >>  6) & 1;
> +    }
> +
>      return 1;
>  }
>  
> @@ -157,7 +178,7 @@ static int apv_read_header(AVFormatContext *s)
>      AVStream *st;
>      GetByteContext gbc;
>      APVHeaderInfo header;
> -    uint8_t buffer[28];
> +    uint8_t buffer[64];

Don't make this buffer unnecessarily big.

>      uint32_t au_size, signature, pbu_size;
>      int err, size;
>  
> @@ -201,6 +222,11 @@ static int apv_read_header(AVFormatContext *s)
>      st->codecpar->level      = header.level_idc;
>      st->codecpar->width      = header.frame_width;
>      st->codecpar->height     = header.frame_height;
> +    st->codecpar->color_primaries = header.color_primaries;
> +    st->codecpar->color_trc       = header.transfer_characteristics;
> +    st->codecpar->color_space     = header.matrix_coefficients;
> +    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
> +                                                           : AVCOL_RANGE_MPEG;
>  
>      st->avg_frame_rate = (AVRational){ 30, 1 };
>      avpriv_set_pts_info(st, 64, 1, 30);

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/apvdec: export color information
  2025-04-27 18:29 ` Mark Thompson
@ 2025-04-27 19:22   ` James Almer
  2025-04-27 19:35   ` [FFmpeg-devel] [PATCH v2] " James Almer
  2025-04-27 19:42   ` [FFmpeg-devel] [PATCH v3] " James Almer
  2 siblings, 0 replies; 8+ messages in thread
From: James Almer @ 2025-04-27 19:22 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 4101 bytes --]

On 4/27/2025 3:29 PM, Mark Thompson wrote:
> On 27/04/2025 18:53, James Almer wrote:
>> apv_read_header() reads enough information that the generic demux code doesn't
>> attempt to read a frame to fill missing fields in codecpar, so make sure it's
>> set here.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/apvdec.c | 32 +++++++++++++++++++++++++++++---
>>   1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
>> index e1ac34b003..94d2ce9965 100644
>> --- a/libavformat/apvdec.c
>> +++ b/libavformat/apvdec.c
>> @@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
>>       uint8_t  chroma_format_idc;
>>       uint8_t  bit_depth_minus8;
>>   
>> +    int      color_primaries;
>> +    int      transfer_characteristics;
>> +    int      matrix_coefficients;
>> +    int      full_range_flag;
>> +
>>       enum AVPixelFormat pixel_format;
>>   } APVHeaderInfo;
>>   
>> @@ -55,6 +60,8 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>>   {
>>       int zero, byte, bit_depth_index;
>>   
>> +    memset(info, 0, sizeof(*info));
>> +
>>       info->pbu_type = bytestream2_get_byte(gbc);
>>       info->group_id = bytestream2_get_be16(gbc);
>>   
>> @@ -107,10 +114,24 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>>       // Ignore capture_time_distance.
>>       bytestream2_skip(gbc, 1);
>>   
>> -    zero = bytestream2_get_byte(gbc);
>> -    if (zero != 0)
>> +    // two reserved_zero_8bits
>> +    if (bytestream2_get_be16(gbc) != 0)
>>           return AVERROR_INVALIDDATA;
>>   
>> +    // color_description_present_flag
>> +    if (bytestream2_peek_byte(gbc) >> 7) {
>> +        // We can read 32 bits as tile info is guaranteed to be present after this.
>> +        unsigned color_description = bytestream2_get_be32(gbc);
>> +
>> +        if (bytestream2_get_bytes_left(gbc) <= 0)
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        info->color_primaries          = (color_description >> 23) & 0xff;
>> +        info->transfer_characteristics = (color_description >> 15) & 0xff;
>> +        info->matrix_coefficients      = (color_description >>  7) & 0xff;
> 
> For these "unspecified" has value 2, which will need to be set somewhere in no-description case rather than zero-init.  (Matrix coefficients 0 means identity, i.e. RGB, which we definitely don't want.)

Ok.

> 
>> +        info->full_range_flag          = (color_description >>  6) & 1;
>> +    }
>> +
>>       return 1;
>>   }
>>   
>> @@ -157,7 +178,7 @@ static int apv_read_header(AVFormatContext *s)
>>       AVStream *st;
>>       GetByteContext gbc;
>>       APVHeaderInfo header;
>> -    uint8_t buffer[28];
>> +    uint8_t buffer[64];
> 
> Could be smaller, but doesn't really matter.

Will make it smaller.

> 
>>       uint32_t au_size, signature, pbu_size;
>>       int err, size;
>>   
>> @@ -201,6 +222,11 @@ static int apv_read_header(AVFormatContext *s)
>>       st->codecpar->level      = header.level_idc;
>>       st->codecpar->width      = header.frame_width;
>>       st->codecpar->height     = header.frame_height;
>> +    st->codecpar->color_primaries = header.color_primaries;
>> +    st->codecpar->color_trc       = header.transfer_characteristics;
>> +    st->codecpar->color_space     = header.matrix_coefficients;
>> +    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
>> +                                                           : AVCOL_RANGE_MPEG;
> 
> Suggest st->codecpar->chroma_location = AVCHROMA_LOC_TOPLEFT as well?

Sure.

> 
>>   
>>       st->avg_frame_rate = (AVRational){ 30, 1 };
>>       avpriv_set_pts_info(st, 64, 1, 30);
> 
> Thanks,
> 
> - Mark
> 
> _______________________________________________
> 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".


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH v2] avformat/apvdec: export color information
  2025-04-27 18:29 ` Mark Thompson
  2025-04-27 19:22   ` James Almer
@ 2025-04-27 19:35   ` James Almer
  2025-04-27 19:42   ` [FFmpeg-devel] [PATCH v3] " James Almer
  2 siblings, 0 replies; 8+ messages in thread
From: James Almer @ 2025-04-27 19:35 UTC (permalink / raw)
  To: ffmpeg-devel

apv_read_header() reads enough information that the generic demux code doesn't
attempt to read a frame to fill missing fields in codecpar, so make sure it's
set here.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/apvdec.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
index e1ac34b003..6a769e51cb 100644
--- a/libavformat/apvdec.c
+++ b/libavformat/apvdec.c
@@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
     uint8_t  chroma_format_idc;
     uint8_t  bit_depth_minus8;
 
+    uint8_t  color_primaries;
+    uint8_t  transfer_characteristics;
+    uint8_t  matrix_coefficients;
+    uint8_t  full_range_flag;
+
     enum AVPixelFormat pixel_format;
 } APVHeaderInfo;
 
@@ -111,6 +116,31 @@ static int apv_extract_header_info(APVHeaderInfo *info,
     if (zero != 0)
         return AVERROR_INVALIDDATA;
 
+    // Return if this function was called by apv_probe()
+    if (bytestream2_get_bytes_left(gbc) == 0)
+        return 1;
+
+    zero = bytestream2_get_byte(gbc);
+    if (zero != 0)
+        return AVERROR_INVALIDDATA;
+
+    // color_description_present_flag
+    if (bytestream2_peek_byte(gbc) >> 7) {
+        unsigned color_description;
+
+        // We can read 32 bits as tile info is guaranteed to be present after this.
+        color_description              = bytestream2_get_be32(gbc);
+        info->color_primaries          = (color_description >> 23) & 0xff;
+        info->transfer_characteristics = (color_description >> 15) & 0xff;
+        info->matrix_coefficients      = (color_description >>  7) & 0xff;
+        info->full_range_flag          = (color_description >>  6) & 1;
+    } else {
+        info->color_primaries          = AVCOL_PRI_UNSPECIFIED;
+        info->transfer_characteristics = AVCOL_TRC_UNSPECIFIED;
+        info->matrix_coefficients      = AVCOL_SPC_UNSPECIFIED;
+        info->full_range_flag          = 0;
+    }
+
     return 1;
 }
 
@@ -157,7 +187,7 @@ static int apv_read_header(AVFormatContext *s)
     AVStream *st;
     GetByteContext gbc;
     APVHeaderInfo header;
-    uint8_t buffer[28];
+    uint8_t buffer[33];
     uint32_t au_size, signature, pbu_size;
     int err, size;
 
@@ -201,6 +231,12 @@ static int apv_read_header(AVFormatContext *s)
     st->codecpar->level      = header.level_idc;
     st->codecpar->width      = header.frame_width;
     st->codecpar->height     = header.frame_height;
+    st->codecpar->chroma_location = AVCHROMA_LOC_TOPLEFT;
+    st->codecpar->color_primaries = header.color_primaries;
+    st->codecpar->color_trc       = header.transfer_characteristics;
+    st->codecpar->color_space     = header.matrix_coefficients;
+    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
+                                                           : AVCOL_RANGE_MPEG;
 
     st->avg_frame_rate = (AVRational){ 30, 1 };
     avpriv_set_pts_info(st, 64, 1, 30);
-- 
2.49.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH v3] avformat/apvdec: export color information
  2025-04-27 18:29 ` Mark Thompson
  2025-04-27 19:22   ` James Almer
  2025-04-27 19:35   ` [FFmpeg-devel] [PATCH v2] " James Almer
@ 2025-04-27 19:42   ` James Almer
  2025-04-27 20:50     ` Andreas Rheinhardt
  2 siblings, 1 reply; 8+ messages in thread
From: James Almer @ 2025-04-27 19:42 UTC (permalink / raw)
  To: ffmpeg-devel

apv_read_header() reads enough information that the generic demux code doesn't
attempt to read a frame to fill missing fields in codecpar, so make sure it's
set here.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/apvdec.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
index e1ac34b003..9f8af35567 100644
--- a/libavformat/apvdec.c
+++ b/libavformat/apvdec.c
@@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
     uint8_t  chroma_format_idc;
     uint8_t  bit_depth_minus8;
 
+    uint8_t  color_primaries;
+    uint8_t  transfer_characteristics;
+    uint8_t  matrix_coefficients;
+    uint8_t  full_range_flag;
+
     enum AVPixelFormat pixel_format;
 } APVHeaderInfo;
 
@@ -111,6 +116,31 @@ static int apv_extract_header_info(APVHeaderInfo *info,
     if (zero != 0)
         return AVERROR_INVALIDDATA;
 
+    // Return if this function was called by apv_probe()
+    if (bytestream2_get_bytes_left(gbc) == 0)
+        return 1;
+
+    zero = bytestream2_get_byte(gbc);
+    if (zero != 0)
+        return AVERROR_INVALIDDATA;
+
+    // color_description_present_flag
+    if (bytestream2_peek_byte(gbc) >> 7) {
+        unsigned color_description;
+
+        // We can read 32 bits as tile info is guaranteed to be present after this.
+        color_description              = bytestream2_get_be32(gbc);
+        info->color_primaries          = (color_description >> 23) & 0xff;
+        info->transfer_characteristics = (color_description >> 15) & 0xff;
+        info->matrix_coefficients      = (color_description >>  7) & 0xff;
+        info->full_range_flag          = (color_description >>  6) & 1;
+    } else {
+        info->color_primaries          = AVCOL_PRI_UNSPECIFIED;
+        info->transfer_characteristics = AVCOL_TRC_UNSPECIFIED;
+        info->matrix_coefficients      = AVCOL_SPC_UNSPECIFIED;
+        info->full_range_flag          = 0;
+    }
+
     return 1;
 }
 
@@ -126,7 +156,7 @@ static int apv_probe(const AVProbeData *p)
         return 0;
     }
 
-    bytestream2_init(&gbc, p->buf, p->buf_size);
+    bytestream2_init(&gbc, p->buf, 28);
 
     au_size = bytestream2_get_be32(&gbc);
     if (au_size < 24) {
@@ -157,7 +187,7 @@ static int apv_read_header(AVFormatContext *s)
     AVStream *st;
     GetByteContext gbc;
     APVHeaderInfo header;
-    uint8_t buffer[28];
+    uint8_t buffer[33];
     uint32_t au_size, signature, pbu_size;
     int err, size;
 
@@ -201,6 +231,12 @@ static int apv_read_header(AVFormatContext *s)
     st->codecpar->level      = header.level_idc;
     st->codecpar->width      = header.frame_width;
     st->codecpar->height     = header.frame_height;
+    st->codecpar->chroma_location = AVCHROMA_LOC_TOPLEFT;
+    st->codecpar->color_primaries = header.color_primaries;
+    st->codecpar->color_trc       = header.transfer_characteristics;
+    st->codecpar->color_space     = header.matrix_coefficients;
+    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
+                                                           : AVCOL_RANGE_MPEG;
 
     st->avg_frame_rate = (AVRational){ 30, 1 };
     avpriv_set_pts_info(st, 64, 1, 30);
-- 
2.49.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] avformat/apvdec: export color information
  2025-04-27 19:42   ` [FFmpeg-devel] [PATCH v3] " James Almer
@ 2025-04-27 20:50     ` Andreas Rheinhardt
  2025-04-27 20:55       ` James Almer
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Rheinhardt @ 2025-04-27 20:50 UTC (permalink / raw)
  To: ffmpeg-devel

James Almer:
> apv_read_header() reads enough information that the generic demux code doesn't
> attempt to read a frame to fill missing fields in codecpar, so make sure it's
> set here.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/apvdec.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
> index e1ac34b003..9f8af35567 100644
> --- a/libavformat/apvdec.c
> +++ b/libavformat/apvdec.c
> @@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
>      uint8_t  chroma_format_idc;
>      uint8_t  bit_depth_minus8;
>  
> +    uint8_t  color_primaries;
> +    uint8_t  transfer_characteristics;
> +    uint8_t  matrix_coefficients;
> +    uint8_t  full_range_flag;
> +
>      enum AVPixelFormat pixel_format;
>  } APVHeaderInfo;
>  
> @@ -111,6 +116,31 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>      if (zero != 0)
>          return AVERROR_INVALIDDATA;
>  
> +    // Return if this function was called by apv_probe()

Why do you want to make probing less strict?

> +    if (bytestream2_get_bytes_left(gbc) == 0)
> +        return 1;
> +
> +    zero = bytestream2_get_byte(gbc);
> +    if (zero != 0)
> +        return AVERROR_INVALIDDATA;
> +
> +    // color_description_present_flag
> +    if (bytestream2_peek_byte(gbc) >> 7) {
> +        unsigned color_description;
> +
> +        // We can read 32 bits as tile info is guaranteed to be present after this.
> +        color_description              = bytestream2_get_be32(gbc);
> +        info->color_primaries          = (color_description >> 23) & 0xff;
> +        info->transfer_characteristics = (color_description >> 15) & 0xff;
> +        info->matrix_coefficients      = (color_description >>  7) & 0xff;
> +        info->full_range_flag          = (color_description >>  6) & 1;
> +    } else {
> +        info->color_primaries          = AVCOL_PRI_UNSPECIFIED;
> +        info->transfer_characteristics = AVCOL_TRC_UNSPECIFIED;
> +        info->matrix_coefficients      = AVCOL_SPC_UNSPECIFIED;
> +        info->full_range_flag          = 0;
> +    }
> +
>      return 1;
>  }
>  
> @@ -126,7 +156,7 @@ static int apv_probe(const AVProbeData *p)
>          return 0;
>      }
>  
> -    bytestream2_init(&gbc, p->buf, p->buf_size);
> +    bytestream2_init(&gbc, p->buf, 28);
>  
>      au_size = bytestream2_get_be32(&gbc);
>      if (au_size < 24) {
> @@ -157,7 +187,7 @@ static int apv_read_header(AVFormatContext *s)
>      AVStream *st;
>      GetByteContext gbc;
>      APVHeaderInfo header;
> -    uint8_t buffer[28];
> +    uint8_t buffer[33];
>      uint32_t au_size, signature, pbu_size;
>      int err, size;
>  
> @@ -201,6 +231,12 @@ static int apv_read_header(AVFormatContext *s)
>      st->codecpar->level      = header.level_idc;
>      st->codecpar->width      = header.frame_width;
>      st->codecpar->height     = header.frame_height;
> +    st->codecpar->chroma_location = AVCHROMA_LOC_TOPLEFT;
> +    st->codecpar->color_primaries = header.color_primaries;
> +    st->codecpar->color_trc       = header.transfer_characteristics;
> +    st->codecpar->color_space     = header.matrix_coefficients;
> +    st->codecpar->color_range     = header.full_range_flag ? AVCOL_RANGE_JPEG
> +                                                           : AVCOL_RANGE_MPEG;
>  
>      st->avg_frame_rate = (AVRational){ 30, 1 };
>      avpriv_set_pts_info(st, 64, 1, 30);

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] avformat/apvdec: export color information
  2025-04-27 20:50     ` Andreas Rheinhardt
@ 2025-04-27 20:55       ` James Almer
  0 siblings, 0 replies; 8+ messages in thread
From: James Almer @ 2025-04-27 20:55 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1429 bytes --]

On 4/27/2025 5:50 PM, Andreas Rheinhardt wrote:
> James Almer:
>> apv_read_header() reads enough information that the generic demux code doesn't
>> attempt to read a frame to fill missing fields in codecpar, so make sure it's
>> set here.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/apvdec.c | 40 ++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
>> index e1ac34b003..9f8af35567 100644
>> --- a/libavformat/apvdec.c
>> +++ b/libavformat/apvdec.c
>> @@ -39,6 +39,11 @@ typedef struct APVHeaderInfo {
>>       uint8_t  chroma_format_idc;
>>       uint8_t  bit_depth_minus8;
>>   
>> +    uint8_t  color_primaries;
>> +    uint8_t  transfer_characteristics;
>> +    uint8_t  matrix_coefficients;
>> +    uint8_t  full_range_flag;
>> +
>>       enum AVPixelFormat pixel_format;
>>   } APVHeaderInfo;
>>   
>> @@ -111,6 +116,31 @@ static int apv_extract_header_info(APVHeaderInfo *info,
>>       if (zero != 0)
>>           return AVERROR_INVALIDDATA;
>>   
>> +    // Return if this function was called by apv_probe()
> 
> Why do you want to make probing less strict?

I'm not making it less strict, just ensuring apv_probe() behaves the 
exact same.
In any case, I'm withdrawing this patch in favor of a more proper 
approach. Will send it in a bit.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-04-27 20:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-27 17:53 [FFmpeg-devel] [PATCH] avformat/apvdec: export color information James Almer
2025-04-27 18:29 ` Mark Thompson
2025-04-27 19:22   ` James Almer
2025-04-27 19:35   ` [FFmpeg-devel] [PATCH v2] " James Almer
2025-04-27 19:42   ` [FFmpeg-devel] [PATCH v3] " James Almer
2025-04-27 20:50     ` Andreas Rheinhardt
2025-04-27 20:55       ` James Almer
2025-04-27 18:57 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git