| Message ID | 20190417154824.5516-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 67f9d3f461f65395f39d3e01801ce8b572d0aa12 |
| Headers | show |
On 17/04/2019 16:48, James Almer wrote: > Also infer the value time_offset_length as 0 when it's not present. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Fun thing, this metadata OBU is clearly based on the H264/5 timecode SEI, yet > time_offset_length is unsigned here :p > > libavcodec/cbs_av1_syntax_template.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > index 59a98b18c9..b04cd51d55 100644 > --- a/libavcodec/cbs_av1_syntax_template.c > +++ b/libavcodec/cbs_av1_syntax_template.c > @@ -1753,19 +1753,19 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, > fb(9, n_frames); > > if (current->full_timestamp_flag) { > - fb(6, seconds_value); > - fb(6, minutes_value); > - fb(5, hours_value); > + fc(6, seconds_value, 0, 59); What, no leap seconds? :P > + fc(6, minutes_value, 0, 59); > + fc(5, hours_value, 0, 23); > } else { > flag(seconds_flag); > if (current->seconds_flag) { > - fb(6, seconds_value); > + fc(6, seconds_value, 0, 59); > flag(minutes_flag); > if (current->minutes_flag) { > - fb(6, minutes_value); > + fc(6, minutes_value, 0, 59); > flag(hours_flag); > if (current->hours_flag) > - fb(5, hours_value); > + fc(5, hours_value, 0, 23); > } > } > } > @@ -1773,6 +1773,8 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, > fb(5, time_offset_length); > if (current->time_offset_length > 0) > fb(current->time_offset_length, time_offset_value); > + else > + infer(time_offset_length, 0); > > return 0; > } > LGTM. Thanks, - Mark
On 4/28/2019 7:28 PM, Mark Thompson wrote: > On 17/04/2019 16:48, James Almer wrote: >> Also infer the value time_offset_length as 0 when it's not present. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Fun thing, this metadata OBU is clearly based on the H264/5 timecode SEI, yet >> time_offset_length is unsigned here :p >> >> libavcodec/cbs_av1_syntax_template.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c >> index 59a98b18c9..b04cd51d55 100644 >> --- a/libavcodec/cbs_av1_syntax_template.c >> +++ b/libavcodec/cbs_av1_syntax_template.c >> @@ -1753,19 +1753,19 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, >> fb(9, n_frames); >> >> if (current->full_timestamp_flag) { >> - fb(6, seconds_value); >> - fb(6, minutes_value); >> - fb(5, hours_value); >> + fc(6, seconds_value, 0, 59); > > What, no leap seconds? :P Not even the Linux Kernel could handle those properly at some point :p > >> + fc(6, minutes_value, 0, 59); >> + fc(5, hours_value, 0, 23); >> } else { >> flag(seconds_flag); >> if (current->seconds_flag) { >> - fb(6, seconds_value); >> + fc(6, seconds_value, 0, 59); >> flag(minutes_flag); >> if (current->minutes_flag) { >> - fb(6, minutes_value); >> + fc(6, minutes_value, 0, 59); >> flag(hours_flag); >> if (current->hours_flag) >> - fb(5, hours_value); >> + fc(5, hours_value, 0, 23); >> } >> } >> } >> @@ -1773,6 +1773,8 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, >> fb(5, time_offset_length); >> if (current->time_offset_length > 0) >> fb(current->time_offset_length, time_offset_value); >> + else >> + infer(time_offset_length, 0); >> >> return 0; >> } >> > > LGTM. Pushed. Look at the h2645 patchset set i sent before this patch as well whenever you can. Thanks!
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index 59a98b18c9..b04cd51d55 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1753,19 +1753,19 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, fb(9, n_frames); if (current->full_timestamp_flag) { - fb(6, seconds_value); - fb(6, minutes_value); - fb(5, hours_value); + fc(6, seconds_value, 0, 59); + fc(6, minutes_value, 0, 59); + fc(5, hours_value, 0, 23); } else { flag(seconds_flag); if (current->seconds_flag) { - fb(6, seconds_value); + fc(6, seconds_value, 0, 59); flag(minutes_flag); if (current->minutes_flag) { - fb(6, minutes_value); + fc(6, minutes_value, 0, 59); flag(hours_flag); if (current->hours_flag) - fb(5, hours_value); + fc(5, hours_value, 0, 23); } } } @@ -1773,6 +1773,8 @@ static int FUNC(metadata_timecode)(CodedBitstreamContext *ctx, RWContext *rw, fb(5, time_offset_length); if (current->time_offset_length > 0) fb(current->time_offset_length, time_offset_value); + else + infer(time_offset_length, 0); return 0; }
Also infer the value time_offset_length as 0 when it's not present. Signed-off-by: James Almer <jamrial@gmail.com> --- Fun thing, this metadata OBU is clearly based on the H264/5 timecode SEI, yet time_offset_length is unsigned here :p libavcodec/cbs_av1_syntax_template.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)