[vlc-devel] Ogg muxer patches

[Denis Charmet]

Hi,

Le dimanche 27 octobre 2013 à 05:46:49, Francois Cartegnie a écrit :
> +static void OggGetSkeletonFisbone( uint8_t **pp_buffer, long *pi_size,
> +                                   sout_input_t *p_input, sout_mux_t *p_mux )
> +{
> +    struct
> +    {
> +        vlc_fourcc_t i_fourcc;
> +        const char * psz_mime;
> +    } codec_to_mime[] =
> +    {
> +        {VLC_CODEC_VORBIS, "audio/vorbis"},
> +        {VLC_CODEC_THEORA, "video/theora"},
> +        {VLC_CODEC_SPEEX,  "audio/speex"},
> +        {VLC_CODEC_FLAC,   "audio/flac"},
> +        {VLC_CODEC_CMML,   "text/cmml"},
> +        {VLC_CODEC_KATE,   "application/kate"},
> +    };
const

> +    uint8_t *psz_header;
> +    const char *psz_value = NULL;
> +    ogg_stream_t *p_stream = (ogg_stream_t *) p_input->p_sys;
> +    struct
> +    {
> +        char * psz_content_type;
> +        char * psz_role;
> +        long int i_size;
> +        unsigned int i_count;
> +    } headers = { NULL, NULL, 0, 0 };
> +    *pi_size = 0;
> +
> +    for ( unsigned int i = 0; i < ARRAY_SIZE(codec_to_mime); i++ )
> +    {
> +        if ( codec_to_mime[i].i_fourcc == p_stream->i_fourcc )
> +        {
> +            psz_value = codec_to_mime[i].psz_mime;
> +            break;
> +        }
> +    }
I'd rather see a switch/case than a loop that should end up unrolled by
the compiler.
> +    if ( psz_value == NULL )
> +    {
> +        psz_value = "application/octet-stream";
> +        msg_Warn( p_mux, "Unkown fourcc for stream %s, setting Content-Type to %s",
> +                  vlc_fourcc_GetDescription( p_stream->i_cat, p_stream->i_fourcc ),
> +                  psz_value );
> +    }
> +
> +    /* Content Type Header */
> +    if ( asprintf( &headers.psz_content_type, "Content-Type: %s\r\n", psz_value ) != -1 )
> +    {
> +        headers.i_size += strlen( headers.psz_content_type );
> +        headers.i_count++;
> +    }
> +
> +    /* Set Role Header */
> +    if ( p_input->p_fmt->i_priority > -2 )
> +    {
> +        int i_max_prio = -2;
> +        for ( int i=0; i< p_mux->i_nb_inputs; i++ )
> +        {
> +            if ( p_mux->pp_inputs[i]->p_fmt->i_cat != p_input->p_fmt->i_cat ) continue;
> +            i_max_prio = __MAX( p_mux->pp_inputs[i]->p_fmt->i_priority, i_max_prio );
> +        }
> +
> +        psz_value = NULL;
> +        if ( p_input->p_fmt->i_cat == AUDIO_ES || p_input->p_fmt->i_cat == VIDEO_ES )
> +        {
> +            if ( p_input->p_fmt->i_priority == i_max_prio && i_max_prio >= 0 )
> +                psz_value = ( p_input->p_fmt->i_cat == VIDEO_ES ) ?
> +                            "video/main" : "audio/main";
> +            else
> +                psz_value = ( p_input->p_fmt->i_cat == VIDEO_ES ) ?
> +                            "video/alternate" : "audio/alternate";
> +        }
> +
> +        if ( p_input->p_fmt->i_cat == SPU_ES )
why not else if ? (once again I prefer switch/case to several if/else
they tend to be optimized better by the compiler)
> +        {
> +            psz_value = ( p_input->p_fmt->i_codec == VLC_CODEC_KATE ) ?
> +                        "text/karaoke" : "text/subtitle";
> +        }
> +
> +        if ( psz_value && asprintf( &headers.psz_role, "Role: %s\r\n", psz_value ) != -1 )
> +        {
> +            headers.i_size += strlen( headers.psz_role );
> +            headers.i_count++;
> +        }
> +    }
> +
> +    *pp_buffer = calloc( FISBONE_BASE_SIZE + headers.i_size, sizeof(uint8_t) );
> +    if ( !*pp_buffer ) return;
> +
> +    memcpy( *pp_buffer, "fisbone", 8 );
> +    SetWLE( &(*pp_buffer)[8], FISBONE_BASE_OFFSET ); /* offset to message headers */
> +    SetWLE( &(*pp_buffer)[12], p_stream->i_serial_no );
> +    SetWLE( &(*pp_buffer)[16], headers.i_count );
> +
> +    /* granulerate den */
> +    switch ( p_input->p_fmt->i_cat )
> +    {
> +        case VIDEO_ES:
> +            SetQWLE( &(*pp_buffer)[20], p_input->p_fmt->video.i_frame_rate );
> +            SetQWLE( &(*pp_buffer)[28], 1000 * p_input->p_fmt->video.i_frame_rate_base );
> +        break;
> +        case AUDIO_ES:
> +            SetQWLE( &(*pp_buffer)[28], p_input->p_fmt->audio.i_rate );
> +        break;
> +        default:
> +            SetQWLE( &(*pp_buffer)[28], 1000 );
> +    }
> +
> +    /* preroll */
> +    if ( p_input->p_fmt->p_extra )
> +        SetWLE( &(*pp_buffer)[44], xiph_CountHeaders( p_input->p_fmt->p_extra ) );
> +
> +    psz_header = *pp_buffer + FISBONE_BASE_SIZE;
> +    memcpy( psz_header, headers.psz_content_type, strlen( headers.psz_content_type ) );
> +    psz_header += strlen( headers.psz_content_type );
> +    if ( headers.psz_role )
> +        memcpy( psz_header, headers.psz_role, strlen( headers.psz_role ) );
> +
> +    *pi_size = FISBONE_BASE_SIZE + headers.i_size;
> +
> +    free( headers.psz_content_type );
> +    free( headers.psz_role );
I don't understand why you use a structure for that, but fair enough.

Regards, 

-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre