[FFmpeg-devel] [PATCH] demux individual program out of MPEG-TS

[Benoit Fouet]

Nico Sabbi wrote:
> Benoit Fouet wrote:
>
>   
>>> +    int i, j, k;
>>> +    int used = 0, discarded = 0;
>>> +    Program_t *p;
>>> +    for(i=0; i<ts->nb_prg; i++) {
>>> +        p = &ts->prg[i];
>>> +        for(j=0; j<p->nb_pids; j++) {
>>> +            if(p->pids[j] != pid)
>>> +                continue;
>>>  
>>>    
>>>
>>>       
>> using if(p->pids[j] == pid) would get rid of the continue
>>  
>>
>>     
>
> but it would create one more level of bracing and nesting,
> making the code less readable
>
>   

i just thought this could help the compiler...

>>>             case 0x48:
>>>                 service_type = get8(&p, p_end);
>>>                 if (service_type < 0)
>>> @@ -676,7 +726,9 @@
>>>                 name = getstr8(&p, p_end);
>>>                 if (!name)
>>>                     break;
>>> -                new_service(ts, sid, provider_name, name);
>>> +                program = av_new_program(ts->stream, sid);
>>> +                if(program)
>>> +                    av_set_program_name(program, provider_name, name);
>>>                 break;
>>>  
>>>    
>>>
>>>       
>> how about:
>> name = getstr8(&p, p_end);
>> if (name) {
>>    AVProgram *program = av_new_program(ts->stream, sid);
>>    if (program)
>>        av_set_program_name(program, provider_name, name);
>> }
>> break;
>>     
>
> as above: there's one more level of nesting.
> Generally I find code with less nesting more readable,
> thus discarding all error cases before making something
> actually useful is the result
>
>   

this one was also to have the program variable definition only when
necessary.

>>> Index: libavformat/avformat.h
>>> ===================================================================
>>> --- libavformat/avformat.h	(revisione 10435)
>>> +++ libavformat/avformat.h	(copia locale)
>>> @@ -345,6 +345,14 @@
>>>     int64_t pts_buffer[MAX_REORDER_DELAY+1];
>>> } AVStream;
>>>
>>> +typedef struct AVProgram {
>>> +    int            id;
>>> +    char*          provider_name; //Network name for DVB streams
>>> +    char*          name;          //Service name for DVB streams
>>> +    int            running;
>>> +    enum AVDiscard discard;       //selects which program to discard and which to feed to the caller
>>> +} AVProgram;
>>> +
>>>  
>>>    
>>>
>>>       
>> comments are not doxygen compatible
>> (and i personnaly slightly prefer "char *name" to "char* name")
>>  
>>
>>     
> such as this?
>
> +typedef struct AVProgram {
> +    int                      id;
> +    char                   *provider_name; //Network name for DVB streams
> +    char                   *name;          //Service name for DVB streams
> +    int                      running;
> +    enum AVDiscard discard;       //selects which program to discard and which to feed to the caller
> +} AVProgram;
>
>
>   

yes, with doxygen comments (///<) and alignment

-- 
Ben
Purple Labs S.A.
www.purplelabs.com