diff mbox series

[FFmpeg-devel,v5,4/4] avformat/mpegts: Refactor DOVI descriptor parsing to use ff_isom_parse_dvcc_dvvc

Message ID 20210927231632.306250-4-tcChlisop0@gmail.com
State New
Headers show
Series [FFmpeg-devel,v5,1/4] avformat/dovi_isom: Implement Dolby Vision configuration parsing/writing | expand

Checks

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

Commit Message

quietvoid Sept. 27, 2021, 11:16 p.m. UTC
Also fixes incorrect parsing of dv_bl_signal_compatibility_id,
which was often set to zero instead of the actual value, for mpegts only.

Signed-off-by: quietvoid <tcChlisop0@gmail.com>
---
 libavformat/mpegts.c | 43 +++----------------------------------------
 1 file changed, 3 insertions(+), 40 deletions(-)

Comments

quietvoid Sept. 28, 2021, 2:13 a.m. UTC | #1
On 27/09/2021 19.16, quietvoid wrote:

> Also fixes incorrect parsing of dv_bl_signal_compatibility_id,
> which was often set to zero instead of the actual value, for mpegts only.
>
> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> ---
>   libavformat/mpegts.c | 43 +++----------------------------------------
>   1 file changed, 3 insertions(+), 40 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index da8eee2414..276864be93 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -37,8 +37,10 @@
>   #include "mpegts.h"
>   #include "internal.h"
>   #include "avio_internal.h"
> +#include "dovi_isom.h"
>   #include "mpeg.h"
>   #include "isom.h"
> +
>   #if CONFIG_ICONV
>   #include <iconv.h>
>   #endif
> @@ -2162,49 +2164,10 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>           break;
>       case 0xb0: /* DOVI video stream descriptor */
>           {
> -            uint32_t buf;
> -            AVDOVIDecoderConfigurationRecord *dovi;
> -            size_t dovi_size;
>               int ret;
> -            if (desc_end - *pp < 4) // (8 + 8 + 7 + 6 + 1 + 1 + 1) / 8
> -                return AVERROR_INVALIDDATA;
>   
> -            dovi = av_dovi_alloc(&dovi_size);
> -            if (!dovi)
> -                return AVERROR(ENOMEM);
> -
> -            dovi->dv_version_major = get8(pp, desc_end);
> -            dovi->dv_version_minor = get8(pp, desc_end);
> -            buf = get16(pp, desc_end);
> -            dovi->dv_profile        = (buf >> 9) & 0x7f;    // 7 bits
> -            dovi->dv_level          = (buf >> 3) & 0x3f;    // 6 bits
> -            dovi->rpu_present_flag  = (buf >> 2) & 0x01;    // 1 bit
> -            dovi->el_present_flag   = (buf >> 1) & 0x01;    // 1 bit
> -            dovi->bl_present_flag   =  buf       & 0x01;    // 1 bit
> -            if (desc_end - *pp >= 20) {  // 4 + 4 * 4
> -                buf = get8(pp, desc_end);
> -                dovi->dv_bl_signal_compatibility_id = (buf >> 4) & 0x0f; // 4 bits
> -            } else {
> -                // 0 stands for None
> -                // Dolby Vision V1.2.93 profiles and levels
> -                dovi->dv_bl_signal_compatibility_id = 0;
> -            }
> -
> -            ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
> -                                          (uint8_t *)dovi, dovi_size);
> -            if (ret < 0) {
> -                av_free(dovi);
> +            if ((ret = ff_isom_parse_dvcc_dvvc(fc, st, *pp, desc_len)) < 0)
>                   return ret;
> -            }
> -
> -            av_log(fc, AV_LOG_TRACE, "DOVI, version: %d.%d, profile: %d, level: %d, "
> -                   "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
> -                   dovi->dv_version_major, dovi->dv_version_minor,
> -                   dovi->dv_profile, dovi->dv_level,
> -                   dovi->rpu_present_flag,
> -                   dovi->el_present_flag,
> -                   dovi->bl_present_flag,
> -                   dovi->dv_bl_signal_compatibility_id);
>           }
>           break;
>       default:

This patch is wrong for MPEG TS.
According to the Dolby specification, there is a dependency_pid
before dv_bl_signal_compatibility_id in the descriptor.

Current parsing behaviour in master is also incorrect.

However, I'm unsure if ff_isom_parse_dvcc_dvvc should
support this, since this case is actually not for ISOM.
diff mbox series

Patch

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index da8eee2414..276864be93 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -37,8 +37,10 @@ 
 #include "mpegts.h"
 #include "internal.h"
 #include "avio_internal.h"
+#include "dovi_isom.h"
 #include "mpeg.h"
 #include "isom.h"
+
 #if CONFIG_ICONV
 #include <iconv.h>
 #endif
@@ -2162,49 +2164,10 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
         break;
     case 0xb0: /* DOVI video stream descriptor */
         {
-            uint32_t buf;
-            AVDOVIDecoderConfigurationRecord *dovi;
-            size_t dovi_size;
             int ret;
-            if (desc_end - *pp < 4) // (8 + 8 + 7 + 6 + 1 + 1 + 1) / 8
-                return AVERROR_INVALIDDATA;
 
-            dovi = av_dovi_alloc(&dovi_size);
-            if (!dovi)
-                return AVERROR(ENOMEM);
-
-            dovi->dv_version_major = get8(pp, desc_end);
-            dovi->dv_version_minor = get8(pp, desc_end);
-            buf = get16(pp, desc_end);
-            dovi->dv_profile        = (buf >> 9) & 0x7f;    // 7 bits
-            dovi->dv_level          = (buf >> 3) & 0x3f;    // 6 bits
-            dovi->rpu_present_flag  = (buf >> 2) & 0x01;    // 1 bit
-            dovi->el_present_flag   = (buf >> 1) & 0x01;    // 1 bit
-            dovi->bl_present_flag   =  buf       & 0x01;    // 1 bit
-            if (desc_end - *pp >= 20) {  // 4 + 4 * 4
-                buf = get8(pp, desc_end);
-                dovi->dv_bl_signal_compatibility_id = (buf >> 4) & 0x0f; // 4 bits
-            } else {
-                // 0 stands for None
-                // Dolby Vision V1.2.93 profiles and levels
-                dovi->dv_bl_signal_compatibility_id = 0;
-            }
-
-            ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
-                                          (uint8_t *)dovi, dovi_size);
-            if (ret < 0) {
-                av_free(dovi);
+            if ((ret = ff_isom_parse_dvcc_dvvc(fc, st, *pp, desc_len)) < 0)
                 return ret;
-            }
-
-            av_log(fc, AV_LOG_TRACE, "DOVI, version: %d.%d, profile: %d, level: %d, "
-                   "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
-                   dovi->dv_version_major, dovi->dv_version_minor,
-                   dovi->dv_profile, dovi->dv_level,
-                   dovi->rpu_present_flag,
-                   dovi->el_present_flag,
-                   dovi->bl_present_flag,
-                   dovi->dv_bl_signal_compatibility_id);
         }
         break;
     default: