[FFmpeg-devel] [PATCH] don't set is_streamed when it's not

[Måns Rullgård]

Rich Felker <dalias at aerifal.cx> writes:

> On Thu, Dec 20, 2007 at 10:33:38AM +0100, Michael Niedermayer wrote:
>> On Thu, Dec 20, 2007 at 01:38:10AM +0100, Aurelien Jacobs wrote:
>> > Michael Niedermayer wrote:
>> > 
>> > > On Thu, Dec 20, 2007 at 12:38:14AM +0100, Aurelien Jacobs wrote:
>> > > > Hi,
>> > > > 
>> > > > Currently ffserver uses url_open_buf() and url_open_dyn_buf() and
>> > > > then set is_streamed in the resulting ByteIOContext.
>> > > > This seems plain wrong. Buffers are really not streamed.
>> > > > Attached patch avoid this. OK ?
>> > > 
>> > > hmm, i dont think so
>> > > Its surely true that the buffer in which the header is written is
>> > > seekable. But the packets following are not that is we cant seek back
>> > > and update the header, this would cause problems as muxers would
>> > > think during header writing that they could seek back later and
>> > > update things ...
>> > 
>> > OK. I now understand why is_streamed is needed here.
>> > So I guess the right way to avoid direct access to ByteIOContext
>> > internals here is to add a url_set_streamed() function to the API ?
>> > Is attached patch OK ?
>> 
>> What is the advantage of having one get and one set function for
>> each field in ByteIOContext compared to direct access to
>> ByteIOContext?  Will you also propose to add such get/set functions
>> for each field in AVCodecContext and AVFormatContext ?
>
> I agree with Michael here. Just have a policy of not reordering fields
> and adding new ones only at the end. Then direct access is possible
> and ABI is never broken. All this wrapper function mess reaks of C++
> and has no practical benefit.

Totally agree.

-- 
M?ns Rullg?rd
mans at mansr.com