On Wed, Jul 08, 2020 at 11:59:17PM +0200, Marton Balint wrote:
> 
> 
> On Wed, 8 Jul 2020, lance.lmw...@gmail.com wrote:
> 
> > On Wed, Jul 01, 2020 at 09:39:58PM +0800, lance.lmw...@gmail.com wrote:
> > > From: Limin Wang <lance.lmw...@gmail.com>
> > > 
> > > Signed-off-by: Limin Wang <lance.lmw...@gmail.com>
> > > ---
> > >  libavdevice/decklink_dec.cpp | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> > > index 82106aa..0fc1489 100644
> > > --- a/libavdevice/decklink_dec.cpp
> > > +++ b/libavdevice/decklink_dec.cpp
> > > @@ -41,6 +41,7 @@ extern "C" {
> > >  #include "libavutil/imgutils.h"
> > >  #include "libavutil/intreadwrite.h"
> > >  #include "libavutil/time.h"
> > > +#include "libavutil/timecode.h"
> > >  #include "libavutil/mathematics.h"
> > >  #include "libavutil/reverse.h"
> > >  #include "avdevice.h"
> > > @@ -778,6 +779,21 @@ HRESULT 
> > > decklink_input_callback::VideoInputFrameArrived(
> > >                          AVDictionary* metadata_dict = NULL;
> > >                          int metadata_len;
> > >                          uint8_t* packed_metadata;
> > > +                        AVTimecode tcr;
> > > +                        uint32_t tc_data;
> > > +                        uint8_t *sd;
> > > +                        int size = sizeof(uint32_t) * 4;
> 
> You can push some of these initializers into the blocks in which they are
> used.

Sure, will fix it.

> 
> > > +
> > > +                        if (av_timecode_init_from_string(&tcr, 
> > > ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
> > > +                            if ((tc_data = 
> > > av_timecode_get_smpte_from_framenum(&tcr, 0)) > 0) {
> 
> av_timecode_get_smpte_from_framenum() always succeeds, so this check is
> wrong.

OK, will fix it.

> 
> Also in theory you could query the color framing flag from the decklink api.

av_timecode_get_smpte_from_framenum() assume the Color Frame flag is zero
always, and the conversion from frame to string didn't support the color frame
flag also, so even we set the flag, it's not used still. Do you insist that
this version must support this feature?

> 
> Regards,
> Marton
> 
> > > +                                sd = av_packet_new_side_data(&pkt, 
> > > AV_PKT_DATA_S12M_TIMECODE, size);
> > > +                                if (sd) {
> > > +                                    AV_WL32(sd, 1); // one TC ;
> > > +                                    AV_WL32(sd + 4, tc_data); // TC;
> > > +                                }
> > > +                            }
> > > +                        }
> > > +
> > >                          if (av_dict_set(&metadata_dict, "timecode", tc, 
> > > AV_DICT_DONT_STRDUP_VAL) >= 0) {
> > >                              packed_metadata = 
> > > av_packet_pack_dictionary(metadata_dict, &metadata_len);
> > >                              av_dict_free(&metadata_dict);
> > > -- 
> > > 1.8.3.1
> > > 
> > 
> > Marton, please help to review and give comments.
> > 
> > -- 
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to