| Message ID | 20181009133204.29686-3-joshdk@obe.tv |
|---|---|
| State | New |
| Headers | show |
> 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
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
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
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(+)