[FFmpeg-devel] [PATCH] avdevice/decklink_dec: extracting and outputing klv from vanc

Zivkovic, Milos zivkovic at teralogics.com
Tue Jun 16 00:10:59 EEST 2020


>
>
>
Thanks, I found the issue.
>

Glad to hear it.

Regards,
Milos


On Mon, Jun 15, 2020 at 10:58 PM Marton Balint <cus at passwd.hu> wrote:

>
>
> On Mon, 15 Jun 2020, Zivkovic, Milos wrote:
>
> > I'll submit the patch with the proposed changes once that build issue is
> > resolved.
> >
> > I'm not having any build issues on multiple compilers, and there's one
> > really weird error in the log that you shared.
> >
> > In file included from /usr/include/c++/7/ext/new_allocator.h:33:0,
> >>                  from
> >> /usr/include/c++/7/x86_64-suse-linux/bits/c++allocator.h:33,
> >>                  from /usr/include/c++/7/bits/allocator.h:46,
> >>                  from /usr/include/c++/7/vector:61,
> >>                  from libavdevice/decklink_dec.cpp:25:
> >> ./new:1:1: error: expected unqualified-id before ‘[’ token
> >>  [PACKET]
> >>  ^
> >>
> >
> > I'd say that basically everything goes to hell due to that error and
> > invalid token [PACKET] located in /usr/include/c++/7/new, on the first
> > line.
> > Do you mind checking the file and confirming that it actually contains
> it?
> > Those kinds of tokens shouldn't really ever be in the standard headers.
> >
> > I created a very simple sample on Compiler Explorer (with GCC 7.5) where
> > token [PACKET] is above the includes.
> > Basically everything after it is treated as an error, just like in the
> > shared log.
> > You can check it out here: https://godbolt.org/z/zTJn_9
>
> Thanks, I found the issue. My local directory had a file called "new" and
> the compiler included that instead of the proper c++ header file
> /usr/include/c++/7/new
>
> Sorry for the noise.
>
> Thanks,
> Marton
>
> > Milos
> >
> >
> >
> > On Mon, Jun 15, 2020 at 9:20 PM Marton Balint <cus at passwd.hu> wrote:
> >
> >>
> >>
> >> On Mon, 15 Jun 2020, Zivkovic, Milos wrote:
> >>
> >> > Hello,
> >> >
> >> > I appreciate the comments.
> >> >
> >> > Missing docs for the new option.
> >> >>
> >> >
> >> > Does this refer to .texi files? I'll update the docs.
> >>
> >> Yes.
> >>
> >> >
> >> >
> >> >> Avoid these if possible. Yes, this is C++ code but only because the
> >> >> decklink
> >> >> API requires it. Also it gives me a compile error, and I am not
> familiar
> >> >> enough
> >> >> with C++ to figure out what is going on
> >> >>
> >> >
> >> > It makes everything much easier (just like using std::atomic for ref
> >> > counting does).
> >> > It also doesn't change the needed version of C++ since std::atomic is
> >> > available in C++11 and above.
> >> > However, that error message is concerning and I'd like to fix it.
> Which
> >> > compiler is it?
> >>
> >> GCC 7.5.0, full compiler log is attached.
> >>
> >> >
> >> > You only use this to avoid calling ->Release() right? If that is the
> >> case,
> >> >> then
> >> >> I'd avoid this as well, I am not sure about the compiler support of
> such
> >> >> templating.
> >> >>
> >> >
> >> > Yes, it's used to avoid manually releasing COM pointers which can
> become
> >> > quite a hassle especially if multiple COM objects are involved.
> >> > Error handling is simplified as well since there's no need to worry
> which
> >> > COM object(s) should be released.
> >> > As far as compiler support goes, this is the most basic C++ template
> >> that's
> >> > supported by basically every C++ compiler in the last 20+ years.
> >> > I'm willing to remove it if you still feel strongly that it's
> >> unnecessary.
> >>
> >> Yes please.
> >>
> >> >
> >> > This interface is only available from DeckLink SDK 10.10 or so, but
> you
> >> >> have not
> >> >> bumped the version requirement in configure.
> >> >>
> >> >
> >> > Wasn't sure which files should be edited for this.
> >> > I'll get the exact SDK version that introduces it and bump it in
> >> configure.
> >> >
> >> > Is the sorting really needed? Aren't they supposed to be sorted in the
> >> >> VANC?
> >> >>
> >> >
> >> > Well, they should be, but it's not guaranteed so this is just a safety
> >> net.
> >> > If it was guaranteed there would be no need for PSC fields.
> >> >
> >> > Maybe avoid the error message and increase ctx->dropped like other
> >> similar
> >> >> code
> >> >> does it.
> >> >>
> >> >
> >> > Will do.
> >> >
> >> > Mixed code and declaration.
> >> >>
> >> >
> >> > I'll move that block below the declaration.
> >> >
> >> > Lose the error message for ENOMEM.
> >> >>
> >> >
> >> > I was just following what the other code that adds streams does.
> >> >
> >> > AV_OPT_TYPE_BOOL
> >> >>
> >> >
> >> > I'll change that asap.
> >> >
> >> > Thanks,
> >> > Milos
> >>
> >> Thanks,
> >> Marton_______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list