[FFmpeg-devel] matroskadec: Adjust content light side data check

Submitted by Vittorio Giovara on Jan. 13, 2019, 5:17 p.m.

Details

Message ID 20190113171724.61410-1-vittorio.giovara@gmail.com
State New
Headers show

Commit Message

Vittorio Giovara Jan. 13, 2019, 5:17 p.m.
While in practice both fields are always initialized, this mimics
what other tools like ffms2, and x265 do more closely.

This work has been sponsored by Tyrell Corporation, for a compensation
of dozen of cents of US dollars.
---
 libavformat/matroskadec.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Carl Eugen Hoyos Jan. 13, 2019, 5:20 p.m.
2019-01-13 18:17 GMT+01:00, Vittorio Giovara <vittorio.giovara@gmail.com>:
> While in practice both fields are always initialized, this mimics
> what other tools like ffms2, and x265 do more closely.

> This work has been sponsored by Tyrell Corporation, for a compensation
> of dozen of cents of US dollars.

Please remove this.

Carl Eugen
James Almer Jan. 13, 2019, 5:20 p.m.
On 1/13/2019 2:17 PM, Vittorio Giovara wrote:
> While in practice both fields are always initialized, this mimics
> what other tools like ffms2, and x265 do more closely.
> 
> This work has been sponsored by Tyrell Corporation, for a compensation
> of dozen of cents of US dollars.
> ---
>  libavformat/matroskadec.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 4ad99db7db..3ff3516c24 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1877,7 +1877,7 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
>              avcodec_chroma_pos_to_enum((color->chroma_siting_horz - 1) << 7,
>                                         (color->chroma_siting_vert - 1) << 7);
>      }
> -    if (color->max_cll && color->max_fall) {
> +    if (color->max_cll || color->max_fall) {
>          size_t size = 0;
>          int ret;
>          AVContentLightMetadata *metadata = av_content_light_metadata_alloc(&size);
> @@ -1891,6 +1891,9 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
>          }
>          metadata->MaxCLL  = color->max_cll;
>          metadata->MaxFALL = color->max_fall;
> +            av_log(NULL, AV_LOG_INFO, "Content Light Level Metadata, "
> +           "MaxCLL=%d, MaxFALL=%d",
> +           metadata->MaxCLL, metadata->MaxFALL);

This is unnecessary. av_dump_format() already prints side data information.

>      }
>  
>      if (has_mastering_primaries || has_mastering_luminance) {
> @@ -3552,6 +3555,8 @@ static int matroska_read_seek(AVFormatContext *s, int stream_index,
>      AVStream *st = s->streams[stream_index];
>      int i, index, index_min;
>  
> +    flags ^= AVSEEK_FLAG_ANY;

Unrelated change?

> +
>      /* Parse the CUES now since we need the index data to seek. */
>      if (matroska->cues_parsing_deferred > 0) {
>          matroska->cues_parsing_deferred = 0;
>
Paul B Mahol Jan. 13, 2019, 5:20 p.m.
On 1/13/19, Vittorio Giovara <vittorio.giovara@gmail.com> wrote:
> While in practice both fields are always initialized, this mimics
> what other tools like ffms2, and x265 do more closely.
>
> This work has been sponsored by Tyrell Corporation, for a compensation
> of dozen of cents of US dollars.
> ---
>  libavformat/matroskadec.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>

Please do not commit jokes to the tree.
Michael Niedermayer Jan. 13, 2019, 8:41 p.m.
On Sun, Jan 13, 2019 at 12:17:24PM -0500, Vittorio Giovara wrote:
> While in practice both fields are always initialized, this mimics
> what other tools like ffms2, and x265 do more closely.
> 
> This work has been sponsored by Tyrell Corporation, for a compensation
> of dozen of cents of US dollars.
> ---
>  libavformat/matroskadec.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

this  breaks fate-seek-mkv-codec-delay 

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4ad99db7db..3ff3516c24 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1877,7 +1877,7 @@  static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
             avcodec_chroma_pos_to_enum((color->chroma_siting_horz - 1) << 7,
                                        (color->chroma_siting_vert - 1) << 7);
     }
-    if (color->max_cll && color->max_fall) {
+    if (color->max_cll || color->max_fall) {
         size_t size = 0;
         int ret;
         AVContentLightMetadata *metadata = av_content_light_metadata_alloc(&size);
@@ -1891,6 +1891,9 @@  static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
         }
         metadata->MaxCLL  = color->max_cll;
         metadata->MaxFALL = color->max_fall;
+            av_log(NULL, AV_LOG_INFO, "Content Light Level Metadata, "
+           "MaxCLL=%d, MaxFALL=%d",
+           metadata->MaxCLL, metadata->MaxFALL);
     }
 
     if (has_mastering_primaries || has_mastering_luminance) {
@@ -3552,6 +3555,8 @@  static int matroska_read_seek(AVFormatContext *s, int stream_index,
     AVStream *st = s->streams[stream_index];
     int i, index, index_min;
 
+    flags ^= AVSEEK_FLAG_ANY;
+
     /* Parse the CUES now since we need the index data to seek. */
     if (matroska->cues_parsing_deferred > 0) {
         matroska->cues_parsing_deferred = 0;