[FFmpeg-devel] MPEG-TS over UDP broken by r15739

[Baptiste Coudurier]

Hi,

Jindrich Makovicka wrote:
> 2008/11/7 Michael Niedermayer <michaelni at gmx.at>:
>> On Thu, Nov 06, 2008 at 10:17:21PM +0100, Jindrich Makovicka wrote:
>>> On Thu, Nov 6, 2008 at 18:28, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>> On Fri, Oct 31, 2008 at 04:31:01PM +0100, Jindrich Makovicka wrote:
>>>>> Hi,
>>>>>
>>>>> revision 15739 broke receiving datagram based inputs like UDP - aviobuf
>>>>> now discards parts of the packets. Maybe it could be fixed by the
>>>>> attached patch that reverts to the old behavior if max_packet_size is
>>>>> specified.
>>>> Well, the bug has to be analyzed first.
>>>> Just "reverting to the old behavior if ..." is really not how we should
>>>> fix and justify fixes.
>>> Now, the code is clearly broken wrt/ datagram inputs because with a
>>> partially filled buffer, recv() can (and will) truncate the received
>>> UDP packets, and the TS demuxer will get garbage on its input. By
>>> reverting to the old behavior, we ensure that the received packet will
>>> fit into the buffer, which is exactly max_packet_size long. As long as
>>> we assume that having one packet in the buffer is enough for format
>>> autodetection, the fix is ok.
>> understood, thanks for explaining.
>> i agree with the idea of your solution but its mildly buggy, probably doing
>> something random with checksuming also its more complex than needed
>>
>> i think
>> simply changing dst at its initialization should be all that is needed for
>> no max_packet_size case.
> 
> Right, this is a lot simpler and works as well.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: aviobuf.c
> ===================================================================
> --- aviobuf.c	(revision 15773)
> +++ aviobuf.c	(working copy)
> @@ -290,7 +290,7 @@
>  
>  static void fill_buffer(ByteIOContext *s)
>  {
> -    uint8_t *dst= s->buf_end - s->buffer < s->buffer_size ? s->buf_ptr : s->buffer;
> +    uint8_t *dst= (s->max_packet_size == 0 && s->buf_end - s->buffer < s->buffer_size) ? s->buf_ptr : s->buffer;
>      int len= s->buffer_size - (dst - s->buffer);
>  
>      assert(s->buf_ptr == s->buf_end);
> 

Useless parentheses ? and !s->max_packet_size ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no