[FFmpeg-devel] [PATCH] First shot at AVCHD seeking via new seeking API

[Ivan Schreter]

Hi Baptiste,

Baptiste Coudurier wrote:
> [...]
>>>
>>> I prefer:
>>>
>>> typedef struct {
>>> ...
>>> } Type;
>>>
>>> In once, remove the useless opaque declaration in seek.h
>>> seek.h is not a public header.
>> True, one can do it that way.
>>
>> OTOH, I learned my lessons regarding separation of concerns. Since the
>> structure is completely internal to seek.c, why should it be visible
>> even to the format implementer guy who is using seek.h routines? He
>> can't care less... But it does eventually create unneeded include
>> dependencies (I didn't check, though).
>>
>> What do you think about that?
>
> I think this is useless if header is not public.
> Using opaque struct can be usefull to prevent ABI breakage, forcing 
> users to use API.
>
> Please move the complete declaration in seek.h
Done.

>
> [...]
> There are absolutely no casts on malloc in the whole FFmpeg tree.
> IIRC it does not produce a warning in C. Please remove the casts.
>
Done.

>> Do you still have some other open points than already discussed last
>> month?
>
> Yes.
Which ones?

>
>> Shall I disable the new seeking code in mpegts, or can we leave it 
>> for now as-is?
>
> I'll test the code a bit more. Please address all comments ASAP and 
> fix it in svn.

I've addressed your comments as well as comments from Diego and sent it 
in in two patches (one for seek.[ch] and one for mpegts.c). I hope I 
didn't forget anything.

>
> No need to disable it yet I think, however I'd like to point that 
> committing this way given the state of the patch and the review is 
> unacceptable.

Again, it was not my intention to commit anything not sufficiently 
reviewed. As I said, I felt that there are no other points from your 
side, since you didn't respond for a month. Michael had some comments 
regarding exact timestamp comparison, which I addressed. So I thought it 
is OK now.

Regards,

Ivan