diff mbox

[FFmpeg-devel,1/2] cbs_h265: Fix Time Code SEI syntax

Message ID 20181201075255.2608-1-andreas.rheinhardt@googlemail.com
State Accepted
Commit 9f588ba5ca283c6b0b0b744085275e6253ba5cc6
Headers show

Commit Message

Andreas Rheinhardt Dec. 1, 2018, 7:52 a.m. UTC
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@googlemail.com>
---
https://trac.ffmpeg.org/ticket/4141#comment:9 contains a sample
containing Time Code SEI messages. Parsing it currently fails
because counting_type is detected as out of range. Of course, it is not.

 libavcodec/cbs_h265_syntax_template.c | 56 ++++++++++++++-------------
 1 file changed, 30 insertions(+), 26 deletions(-)

Comments

Mark Thompson Dec. 2, 2018, 7:58 p.m. UTC | #1
On 01/12/2018 07:52, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@googlemail.com>
> ---
> https://trac.ffmpeg.org/ticket/4141#comment:9 contains a sample
> containing Time Code SEI messages. Parsing it currently fails
> because counting_type is detected as out of range. Of course, it is not.
> 
>  libavcodec/cbs_h265_syntax_template.c | 56 ++++++++++++++-------------
>  1 file changed, 30 insertions(+), 26 deletions(-)

Oops.  Yep, applied.

Do you happen to have a sample which could be included as a fate test?  Most of the other SEI messages are found in the standard conformance test suite and therefore get tested by it, but this one isn't.

Thanks,

- Mark
Andreas Rheinhardt Dec. 2, 2018, 8:51 p.m. UTC | #2
Mark Thompson:
> On 01/12/2018 07:52, Andreas Rheinhardt wrote:
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@googlemail.com>
>> ---
>> https://trac.ffmpeg.org/ticket/4141#comment:9 contains a sample
>> containing Time Code SEI messages. Parsing it currently fails
>> because counting_type is detected as out of range. Of course, it is not.
>>
>>  libavcodec/cbs_h265_syntax_template.c | 56 ++++++++++++++-------------
>>  1 file changed, 30 insertions(+), 26 deletions(-)
> 
> Oops.  Yep, applied.
> 
> Do you happen to have a sample which could be included as a fate test?  Most of the other SEI messages are found in the standard conformance test suite and therefore get tested by it, but this one isn't.
> 
> Thanks,
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
As has already been said in the quoted section: Ticket 4141 contains
such a sample. This is also the sample that made me investigate this.

- Andreas
Mark Thompson Dec. 2, 2018, 9:24 p.m. UTC | #3
On 02/12/2018 20:51, Andreas Rheinhardt wrote:
> Mark Thompson:
>> On 01/12/2018 07:52, Andreas Rheinhardt wrote:
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@googlemail.com>
>>> ---
>>> https://trac.ffmpeg.org/ticket/4141#comment:9 contains a sample
>>> containing Time Code SEI messages. Parsing it currently fails
>>> because counting_type is detected as out of range. Of course, it is not.
>>>
>>>  libavcodec/cbs_h265_syntax_template.c | 56 ++++++++++++++-------------
>>>  1 file changed, 30 insertions(+), 26 deletions(-)
>>
>> Oops.  Yep, applied.
>>
>> Do you happen to have a sample which could be included as a fate test?  Most of the other SEI messages are found in the standard conformance test suite and therefore get tested by it, but this one isn't.
>>
> As has already been said in the quoted section: Ticket 4141 contains
> such a sample. This is also the sample that made me investigate this.

Indeed, and I used it for testing.  However, it looks like a TV capture and it was uploaded by someone unknown, so I don't think I can infer anything about its copyright status for inclusion.  I've asked in the ticket.

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
index 0a430df23a..f1e1bb0e7e 100644
--- a/libavcodec/cbs_h265_syntax_template.c
+++ b/libavcodec/cbs_h265_syntax_template.c
@@ -1955,36 +1955,40 @@  static int FUNC(sei_time_code)(CodedBitstreamContext *ctx, RWContext *rw,
     u(2, num_clock_ts, 1, 3);
 
     for (i = 0; i < current->num_clock_ts; i++) {
-        flags(units_field_based_flag[i], 1, i);
-        us(5, counting_type[i], 0, 6,    1, i);
-        flags(full_timestamp_flag[i],    1, i);
-        flags(discontinuity_flag[i],     1, i);
-        flags(cnt_dropped_flag[i],       1, i);
-
-        us(9, n_frames[i], 0, MAX_UINT_BITS(9), 1, i);
-
-        if (current->full_timestamp_flag[i]) {
-            us(6, seconds_value[i], 0, 59, 1, i);
-            us(6, minutes_value[i], 0, 59, 1, i);
-            us(5, hours_value[i],   0, 23, 1, i);
-        } else {
-            flags(seconds_flag[i], 1, i);
-            if (current->seconds_flag[i]) {
+        flags(clock_timestamp_flag[i],   1, i);
+
+        if (current->clock_timestamp_flag[i]) {
+            flags(units_field_based_flag[i], 1, i);
+            us(5, counting_type[i], 0, 6,    1, i);
+            flags(full_timestamp_flag[i],    1, i);
+            flags(discontinuity_flag[i],     1, i);
+            flags(cnt_dropped_flag[i],       1, i);
+
+            us(9, n_frames[i], 0, MAX_UINT_BITS(9), 1, i);
+
+            if (current->full_timestamp_flag[i]) {
                 us(6, seconds_value[i], 0, 59, 1, i);
-                flags(minutes_flag[i], 1, i);
-                if (current->minutes_flag[i]) {
-                    us(6, minutes_value[i], 0, 59, 1, i);
-                    flags(hours_flag[i], 1, i);
-                    if (current->hours_flag[i])
-                        us(5, hours_value[i], 0, 23, 1, i);
+                us(6, minutes_value[i], 0, 59, 1, i);
+                us(5, hours_value[i],   0, 23, 1, i);
+            } else {
+                flags(seconds_flag[i], 1, i);
+                if (current->seconds_flag[i]) {
+                    us(6, seconds_value[i], 0, 59, 1, i);
+                    flags(minutes_flag[i], 1, i);
+                    if (current->minutes_flag[i]) {
+                        us(6, minutes_value[i], 0, 59, 1, i);
+                        flags(hours_flag[i], 1, i);
+                        if (current->hours_flag[i])
+                            us(5, hours_value[i], 0, 23, 1, i);
+                    }
                 }
             }
-        }
 
-        us(5, time_offset_length[i], 0, 31, 1, i);
-        if (current->time_offset_length[i] > 0)
-            us(current->time_offset_length[i], time_offset_value[i],
-               0, MAX_UINT_BITS(current->time_offset_length[i]), 1, i);
+            us(5, time_offset_length[i], 0, 31, 1, i);
+            if (current->time_offset_length[i] > 0)
+                us(current->time_offset_length[i], time_offset_value[i],
+                   0, MAX_UINT_BITS(current->time_offset_length[i]), 1, i);
+        }
     }
 
     return 0;