[FFmpeg-devel] [PATCH] dshow: allow for more codecs take 2

[Roger Pack]

On 2/14/13, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> On Thu, Feb 14, 2013 at 9:39 PM, Roger Pack <rogerdpack2 at gmail.com> wrote:
>> On 2/13/13, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>>> On Wed, Feb 13, 2013 at 11:07 PM, Michael Niedermayer <michaelni at gmx.at>
>>> wrote:
>>>> On Sat, Feb 09, 2013 at 04:11:09AM +0100, Michael Niedermayer wrote:
>>>>> On Fri, Feb 08, 2013 at 06:18:51PM -0700, Roger Pack wrote:
>>>>> > On 2/7/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>> > > On Wed, Feb 06, 2013 at 08:19:01PM +0100, Michael Niedermayer
>>>>> > > wrote:
>>>>> > >> On Wed, Feb 06, 2013 at 10:55:53AM -0700, Roger Pack wrote:
>>>>> > >> > On 2/5/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>> > >> > > On Tue, Feb 05, 2013 at 02:24:42PM -0700, Roger Pack wrote:
>>>>> > >> > >> On 2/5/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>> > >> > >> > On Tue, Feb 05, 2013 at 10:05:52AM -0700, Roger Pack
>>>>> > >> > >> > wrote:
>>>>> > >> > >> >> On 2/4/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>> > >> > >> >> > On Mon, Feb 04, 2013 at 01:50:11PM -0700, Roger Pack
>>>>> > >> > >> >> > wrote:
>>>>> > >> > >> >> >> On 1/25/13, Roger Pack <rogerdpack2 at gmail.com> wrote:
>>>>> > >> > >> >> >> > Ok after the previous discussion
>>>>> > >> > >> >> >> > http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2012-October/131942.html
>>>>> > >> > >> >> >> > it appears the first patch attached (0001 --
>>>>> > >> > >> >> >> > re-attached here
>>>>> > >> > >> >> >> > and
>>>>> > >> > >> >> >> > modified to apply cleaning to git master) is ok.
>>>>> > >> > >> >> >> >
>>>>> > >> > >> >> >> > Also attached is a second patch for better error
>>>>> > >> > >> >> >> > logging
>>>>> > >> > >> >> >> > messages.
>>>>> > >> > >> >> >> > Thank you for your consideration in their regard.
>>>>> > >> > >> >> >>
>>>>> > >> > >> >> >> bump for these 2.  Or maybe you could give Ramiro
>>>>> > >> > >> >> >> commit
>>>>> > >> > >> >> >> rights
>>>>> > >> > >> >> >> I'm
>>>>> > >> > >> >> >> sure he wouldn't abuse :)
>>>>> > >> > >> >> >
>>>>> > >> > >> >> > ramiro has commit rights, he also has a github repo
>>>>> > >> > >> >> >
>>>>> > >> > >> >> > you are dshow maintainer, thus if you want me to merge
>>>>> > >> > >> >> > some git
>>>>> > >> > >> >> > repo/branch just say which ...
>>>>> > >> > >> >>
>>>>> > >> > >> >> Ok please merge the https://github.com/rdp/ffmpeg
>>>>> > >> > >> >> "combined"
>>>>> > >> > >> >> branch
>>>>> > >> > >> >> (Ramiro or Michael maybe whoever gets to it first).
>>>>> > >> > >> >> Thank you.
>>>>> > >> > >> >
>>>>> > >> > >> > merged
>>>>> > >> > >> >
>>>>> > >> > >> > thanks
>>>>> > >> > >>
>>>>> > >> > >> Thanks!
>>>>> > >> > >>
>>>>> > >> > >> I am something of a novice to  this, and appears I made a
>>>>> > >> > >> small
>>>>> > >> > >> mistake and the "combined" branch was lacking a commit or 2,
>>>>> > >> > >> could
>>>>> > >> > >> you
>>>>> > >> > >> pull and merge it again for me?
>>>>> > >> > >> Sorry for the inconvenience.  Hopefully I'll get some
>>>>> > >> > >> experience
>>>>> > >> > >> with
>>>>> > >> > >> this better..
>>>>> > >> > >
>>>>> > >> > > the combined branch is now in bad shape
>>>>> > >> > > If i do merge it, it would result in below (more merge
>>>>> > >> > > commits
>>>>> > >> > > than
>>>>> > >> > > non merge commits):
>>>>> > >> > >
>>>>> > >> > > I suggest you start a new branch and make sure you dont add
>>>>> > >> > > merge
>>>>> > >> > > commits into that branch, you dont need any ATM
>>>>> > >> > > simply cherry pick all commits you need, then look at the
>>>>> > >> > > changes
>>>>> > >> > > and make sure they are ok and test them to make sure they
>>>>> > >> > > work
>>>>> > >> > > then tell me to merge
>>>>> > >> >
>>>>> > >> > Oops didn't realize git tracks merges as well.
>>>>> > >> > Please then check the branch combined2 for appropriateness and
>>>>> > >> > merge
>>>>> > >> > if appropriate.
>>>>> > >>
>>>>> > >> merged
>>>>> > >>
>>>>> > >> thanks
>>>>> > >
>>>>> > > this (or possibly another commit) broke msvc10
>>>>> > > see:
>>>>> > > http://fate.ffmpeg.org/report.cgi?time=20130207203228&slot=x86_32-msvc10-dll-windows-native
>>>>> >
>>>>> > Inexperience shows its face again.  Shared builds were broken (mingw
>>>>> > too).  I've set up my build system to catch them more easily next
>>>>> > time.
>>>>> > I believe I've figured them out (at least for mingw cross compile it
>>>>> > works now) if you could merge rdp/dshow_shared.
>>>>> > Thanks for your patience.
>>>>>
>>>>> merged
>>>>>
>>>>> thanks
>>>>
>>>> still seems to not work correctly:
>>>>
>>>> <BBB-work> someone broke msvc dll builds by marking ff_codec_bmp_tags[]
>>>> with av_export
>>>> <BBB-work> can whoever did that fix it?
>>>> <BBB-work> https://github.com/libav/c99-to-c89/issues/5
>>>>
>>>>
>>>> [...]
>>>
>>> This is also covered by a fate instance:
>>> http://fate.ffmpeg.org/history.cgi?slot=x86_32-msvc10-dll-windows-native
>>>
>>> Shared tables cannot be used as a constant initializer.
>>
>> So what I'm understanding is that the following this is a feature of
>> c99: "Shared tables used as a constant initializer" (i.e. a c99-c89
>> code converter can't overcome it trivially?) so when I made that
>> particular variable shared, it introduced it (by making the table
>> shared).  I guess that makes sense.  Sorry for introducing the c99
>> feature accidentally.
>
> This has nothing to do with C99 or C89.
> Its just a quirk between how importing data symbols works in MSVC and
> how av_export is defined.
>
> When you want to import a data symbol in MSVC, you have to mark the
> definition with declspec(dllimport). Doing this, it basically adds
> another indirection to the struct, which results in it not being able
> to be used in constant initializers anymore.
> However, the usual way to mark such symbols in headers is to have a
> macro which at compile time is "declspec(dllexport)" (or empty if you
> export it through other means), and when using the header to link
> against the lib, the macro is defined to "declspec(dllimport)".
>
> The problem here is that av_export is too dumb. Its always
> "declspec(dllimport)", so the symbols get converted into a imported
> symbol even if they are in the same library, and because the compiler
> doesn't know where the linker will find the symbol, it'll add the
> extra indirection, breaking constant initiliazers.
> av_export was defined this way "on purpose", to keep it simple,
> because the cross-library deps between different libs in the same
> project make defining a "correct" macro for this task a bit more
> complex, so this caveat was an accepted compromise.

Ok that makes sense now thanks for the clarification.