[FFmpeg-devel,2/3] lavf/timecode: document SMPTE struct

Submitted by joshdk@ob-encoder.com on Oct. 9, 2018, 1:32 p.m.

Details

Message ID 20181009133204.29686-3-joshdk@obe.tv
State New
Headers show

Commit Message

joshdk@ob-encoder.com Oct. 9, 2018, 1:32 p.m.
From: Devin Heitmueller <dheitmueller@ltnglobal.com>

There are a number of different binary representations in which
SMPTE timecodes can use.  Make clear that the specific representation
that ffmpeg refers to corresponds to the DV video spec, which is
SMPTE S314M:2005 for standard definition video and ST 370-2013 for
high definition video.
---
 libavutil/timecode.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Devin Heitmueller Oct. 9, 2018, 8:21 p.m.
> On Oct 9, 2018, at 4:07 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 
> 2018-10-09 15:32 GMT+02:00, joshdk@ob-encoder.com <joshdk@ob-encoder.com>:
>> From: Devin Heitmueller <dheitmueller@ltnglobal.com>
>> 
>> There are a number of different binary representations in which
>> SMPTE timecodes can use.  Make clear that the specific representation
>> that ffmpeg refers to corresponds to the DV video spec, which is
>> SMPTE S314M:2005 for standard definition video and ST 370-2013 for
>> high definition video.
> 
> If this is correct - I have no idea - why is only one standard
> mentioned in the actual change?
> 
>> +    /* See SMPTE ST 314M-2005 Sec 4.4.2.2.1 "Time code pack (TC)" */
> 

Yeah, I noticed this after I did the original commit and planned to fix it before I submitted the patch upstream.  Was reminded of it myself when I saw Josh submitted the patch on my behalf.

The information describing the structure is identical between the two specs, and thus referring to both doesn’t give you any additional information.  However it’s possible that someone has access to one spec but not the other (since they are not freely available), and thus referring to both specs probably makes sense.

Josh, feel free to update the patch to refer to both specifications, or drop the patch from your series and I’ll include an updated patch in my next patch series.  Whatever works best for you.

Devin

---
Devin Heitmueller - LTN Global Communications
dheitmueller@ltnglobal.com
joshdk@ob-encoder.com Oct. 11, 2018, 1:15 p.m.
On 9 October 2018 at 21:21, Devin Heitmueller
<dheitmueller@ltnglobal.com> wrote:
>
>> On Oct 9, 2018, at 4:07 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>
>> 2018-10-09 15:32 GMT+02:00, joshdk@ob-encoder.com <joshdk@ob-encoder.com>:
>>> From: Devin Heitmueller <dheitmueller@ltnglobal.com>
>>>
>>> There are a number of different binary representations in which
>>> SMPTE timecodes can use.  Make clear that the specific representation
>>> that ffmpeg refers to corresponds to the DV video spec, which is
>>> SMPTE S314M:2005 for standard definition video and ST 370-2013 for
>>> high definition video.
>>
>> If this is correct - I have no idea - why is only one standard
>> mentioned in the actual change?
>>
>>> +    /* See SMPTE ST 314M-2005 Sec 4.4.2.2.1 "Time code pack (TC)" */
>>
>
> Yeah, I noticed this after I did the original commit and planned to fix it before I submitted the patch upstream.  Was reminded of it myself when I saw Josh submitted the patch on my behalf.
>
> The information describing the structure is identical between the two specs, and thus referring to both doesn’t give you any additional information.  However it’s possible that someone has access to one spec but not the other (since they are not freely available), and thus referring to both specs probably makes sense.
>
> Josh, feel free to update the patch to refer to both specifications, or drop the patch from your series and I’ll include an updated patch in my next patch series.  Whatever works best for you.
>
> Devin

I don't have access to either specs, so I couldn't reference it even
if I wanted to. I'll drop it from this set and you can include it in
your next one.

Josh

Patch hide | download patch | download mbox

diff --git a/libavutil/timecode.c b/libavutil/timecode.c
index 60077ba0c0..5b2bf85caa 100644
--- a/libavutil/timecode.c
+++ b/libavutil/timecode.c
@@ -117,6 +117,7 @@  static unsigned bcd2uint(uint8_t bcd)
 
 char *av_timecode_make_smpte_tc_string(char *buf, uint32_t tcsmpte, int prevent_df)
 {
+    /* See SMPTE ST 314M-2005 Sec 4.4.2.2.1 "Time code pack (TC)" */
     unsigned hh   = bcd2uint(tcsmpte     & 0x3f);    // 6-bit hours
     unsigned mm   = bcd2uint(tcsmpte>>8  & 0x7f);    // 7-bit minutes
     unsigned ss   = bcd2uint(tcsmpte>>16 & 0x7f);    // 7-bit seconds