[FFmpeg-cvslog] r13685 - trunk/ffmpeg.c

[Robert Swain]

2008/6/19 Baptiste Coudurier <baptiste.coudurier at smartjog.com>:
> Michael Niedermayer wrote:
>> On Thu, Jun 19, 2008 at 12:56:45PM +0100, Robert Swain wrote:
>>> 2008/6/17 Michael Niedermayer <michaelni at gmx.at>:
>>>> On Tue, Jun 17, 2008 at 04:00:47AM +0200, Michael Niedermayer wrote:
>>>>> On Tue, Jun 17, 2008 at 02:28:03AM +0100, Robert Swain wrote:
>>>>>> 2008/6/8 Robert Swain <robert.swain at gmail.com>:
>>>>>>> 2008/6/7 Baptiste Coudurier <baptiste.coudurier at smartjog.com>:
>>>>>>>> Any volunteer to purge -target and puts svcd/vcd/dvd.ffpreset,
>>>>>>>> and add ipod/iphone.ffpreset ?
>>>>>> Removing -target may be more difficult as it doesn't just deal with
>>>>>> codec options...
>>>>>>
>>>>>>>> I think ipod is definitely wanted to produce h264 files playable :>
>>>>>>> I intend to work on this as well as PSP, MPEG profiles/levels and
>>>>>>> probably some libx264 quality preset stuff too. How do we want to
>>>>>>> manage the presets? Dump them in a presets subdir of FFmpeg trunk or
>>>>>>> something? Then how will we install them? Wouldn't it be better to
>>>>>>> have them installed to /usr/share or something but read files from
>>>>>>> ~/.ffmpeg preferentially?
>>>>>> I've encountered a problem somewhere but I'm not quite sure where. At
>>>>>> the moment I'm suspecting opt_default but that's as far as I've got.
>>>>>> If I have a line in a preset file that reads (without the quotes):
>>>>>> 'rc_eq=blurCplx^(1-qComp)' opt_preset parses it from the file
>>>>>> correctly and passes it to opt_default correctly but when I check the
>>>>>> value fo avctx->rc_eq in libx264.c it is '\003' and nothing more. Any
>>>>>> idea why that might happen? Just putting it out there in case you spot
>>>>>> the issue quicker than I do. :) *Continues the hunt...*
>>>>> try av_strdup() the argument, sorry this is my fault ...
>>>>> ill fix it tomorrow unless you are quicker
>>>> Iam not sure which is the best solution ...
>>>> we could av_strdup() in av_set_string() (instead of the memcpy)
>>>> or
>>>> add a av_set_string_dup() that does the av_strdup() when the field is a
>>>> char *
>>>> or
>>>> check before the av_set_string() call if the field is a char * and do a
>>>> av_strdup() when the source is static.
>>>>
>>>> Also theres the issue with freeing them again. If we do plan to free
>>>> them again (is actually not really needed for ffmpeg.c) then we must
>>>> ensure that we dont mix pointers to the command line with malloced()
>>>> memory ...
>>> Pros and cons of each? The first approach sounds quite simple and
>>> minimal in terms of code alteration.
>>
>> The first breaks the API & ABI and we loose the ability to set char* to
>> a pointer, as its always set to a copy, i do not know if any of these is an
>> issue in practice. But AVOptions originally where supposed to be just some
>> lightwight access layer to access fields of structures, enumerate them, ...
>>
>> It was reimar who added malloc() for FF_OPT_TYPE_BINARY and broke the
>> lightweightness of AVOptions with that.
>> Id really like to drop FF_OPT_TYPE_BINARY and let codecs parse a hex
>> string rather it seems much cleaner and simpler ...
>>
>> Maybe we should just add a av_set_string2(..., int flags)
>> where flags indicate if strdup and free should be done on *char
>> That could then at least treat FF_OPT_TYPE_BINARY without malloc/free
>> flags as error.
>>
>
> I'd like to drop malloc/free problem for sure, but I'd like to keep a
> simple and easy way to set decryption keys.
>
> Maybe an helper in opt.c doing conversion would help, and a way to set
> key len has to be found too.

Any progress with any of this stuff?

Rob