[vlc-devel] [PATCH] Handling prefered language in AsfSelectStream Better, bitrate sort and select algo Tested with asf containing 3, Videos and 24 Lang...
Jean-Baptiste Kempf
jb at videolan.org
Fri Jul 12 12:43:16 CEST 2013
On 10 Jul, Alain Degreffe wrote :
> index d353bfd..916f994 100644
> --- a/modules/access/mms/asf.c
> +++ b/modules/access/mms/asf.c
> @@ -46,6 +46,8 @@ void asf_HeaderParse ( asf_header_t *hdr,
> var_buffer_t buffer;
> guid_t guid;
> uint64_t i_size;
> + uint16_t stream_number;
> + uint16_t lang_ID;
Please align vertically.
> + else if( guidcmp( &guid, &asf_object_language_list ) )
> + {
> + int i_count;
> + uint64_t i_langid_len;
> + char * s_lang_str;
> + int i_current_id;
> + i_count = var_buffer_get16( &buffer );
> +
> + for( i_current_id = 0 ; i_current_id < i_count ;
> i_current_id++ )
> + {
> + i_langid_len = var_buffer_get8( &buffer );
> + s_lang_str = malloc( i_langid_len/2 );
> + if( !s_lang_str )
> + {
> + /* Malloc problem - Zap the remaining bytes of
> object */
> + var_buffer_getmemory( &buffer, NULL, i_langid_len );
> + continue;
> + }
> + uint64_t i;
> +
> + for( i = 0 ; i < i_langid_len ; i = i + 2 )
> + {
> + s_lang_str[i/2] = GetWLE( var_buffer_get16(
> &buffer ) );
> + }
> + s_lang_str[i/2] = '\0';
> + hdr->tablang[i_current_id] = malloc( i/2 );
> + if( hdr->tablang[i_current_id] )
> + {
> + memcpy( hdr->tablang[i_current_id],
> s_lang_str,i/2 + 1 );
> + }
> + free( s_lang_str );
I really think this should be in a separate (static inlined?) function
> -void asf_StreamSelect ( asf_header_t *hdr,
> +void asf_ClearCandidate ( asf_candidate_t *cell )
> +{
> + asf_candidate_t * tmp;
> + while( cell->is_next )
> + {
> + tmp = cell ;
> + cell = tmp->next;
> + free(tmp);
> + }
> + free(cell);
> +}
See the remark down about is_next.
> +asf_candidate_t * asf_AddCandidate( asf_candidate_t
> *work_Candidate, int stream_candidate_id )
> +{
> + work_Candidate->value = stream_candidate_id;
> + work_Candidate->is_next = true;
> + work_Candidate->next = malloc( sizeof( asf_candidate_t ) );
> + if( !work_Candidate->next ) return work_Candidate;
> + work_Candidate = work_Candidate->next;
> + work_Candidate->is_next = false;
> + return work_Candidate;
> +}
idem.
> +void asf_StreamSelect ( asf_header_t *hdr,
> int i_bitrate_max,
> - bool b_all, bool b_audio, bool b_video )
> + bool b_all, bool b_audio, bool
> b_video , char * lang )
> {
> /* XXX FIXME use mututal eclusion information */
> unsigned i;
> int i_audio, i_video;
> - int i_bitrate_total;
> + int i_bitrate_var, i_bitrate_total;
> #if 0
> char *psz_stream;
> #endif
>
> - i_audio = 0;
> - i_video = 0;
> - i_bitrate_total = 0;
> if( b_all )
> {
> /* select all valid stream */
> @@ -214,78 +277,97 @@ void asf_StreamSelect ( asf_header_t *hdr,
> }
> }
>
> - /* big test:
> - * select a stream if
> - * - no audio nor video stream
> - * - or:
> - * - if i_bitrate_max not set keep the highest bitrate
> - * - if i_bitrate_max is set, keep stream that make we
> used best
> - * quality regarding i_bitrate_max
> - *
> - * XXX: little buggy:
> - * - it doesn't use mutual exclusion info..
> - * - when selecting a better stream we could select
> - * something that make i_bitrate_total> i_bitrate_max
> - */
> - for( i = 1; i < 128; i++ )
> + i_audio = -1;
> + i_video = -1;
> + i_bitrate_total = 0;
> + i_bitrate_var = 0;
> +
> + asf_candidate_t * head_Candidatefavlang, * courant_Candidatefavlang;
> + asf_candidate_t * head_Candidateotherlang, *
> courant_Candidateotherlang;
> + asf_candidate_t * head_CandidateVideo, * courant_CandidateVideo;
> + asf_candidate_t * work_Candidate;
> +
> + head_Candidatefavlang = malloc( sizeof( asf_candidate_t ) );
> + if( !head_Candidatefavlang ) return;
> + head_Candidatefavlang->is_next = false;
> + courant_Candidatefavlang = head_Candidatefavlang;
> +
> + head_Candidateotherlang = malloc( sizeof( asf_candidate_t ) );
> + if( !head_Candidatefavlang ) return;
> + head_Candidateotherlang->is_next = false;
> + courant_Candidateotherlang = head_Candidateotherlang;
> +
> + head_CandidateVideo = malloc( sizeof( asf_candidate_t ) );
> + if( !head_Candidatefavlang ) return;
You could group and check all the malloc issues together.
> + courant_CandidateVideo = head_CandidateVideo;
> +
> + for( i = 1 ; i < 128 ; i++ )
> {
> if( hdr->stream[i].i_cat == ASF_CODEC_TYPE_UNKNOWN )
> {
> continue;
> }
> - else if( hdr->stream[i].i_cat == ASF_CODEC_TYPE_AUDIO && b_audio &&
> - ( i_audio <= 0 ||
> - ( ( ( hdr->stream[i].i_bitrate >
> hdr->stream[i_audio].i_bitrate &&
> - ( i_bitrate_total +
> hdr->stream[i].i_bitrate - hdr->stream[i_audio].i_bitrate
> - < i_bitrate_max ||
> !i_bitrate_max) ) ||
> - ( hdr->stream[i].i_bitrate <
> hdr->stream[i_audio].i_bitrate &&
> - i_bitrate_max != 0 && i_bitrate_total
> > i_bitrate_max )
> - ) ) ) )
> +
> + /* audio */
> + if( hdr->stream[i].i_cat == ASF_CODEC_TYPE_AUDIO && b_audio
> && ( hdr->stream[i].i_bitrate < i_bitrate_max || !i_bitrate_max ) )
> {
> - /* unselect old stream */
> - if( i_audio > 0 )
> + if( lang != NULL && strcmp(
> hdr->tablang[hdr->stream[i].i_lang_id], lang ) == 0 )
> {
> - hdr->stream[i_audio].i_selected = 0;
> - if( hdr->stream[i_audio].i_bitrate> 0 )
> - {
> - i_bitrate_total -= hdr->stream[i_audio].i_bitrate;
> - }
> + courant_Candidatefavlang =
> asf_AddCandidate(courant_Candidatefavlang, i);
> }
> -
> - hdr->stream[i].i_selected = 1;
> - if( hdr->stream[i].i_bitrate> 0 )
> + else
> {
> - i_bitrate_total += hdr->stream[i].i_bitrate;
> + courant_Candidateotherlang =
> asf_AddCandidate(courant_Candidateotherlang, i);
> }
> - i_audio = i;
> }
> - else if( hdr->stream[i].i_cat == ASF_CODEC_TYPE_VIDEO && b_video &&
> - ( i_video <= 0 ||
> - (
> - ( ( hdr->stream[i].i_bitrate >
> hdr->stream[i_video].i_bitrate &&
> - ( i_bitrate_total +
> hdr->stream[i].i_bitrate - hdr->stream[i_video].i_bitrate
> - < i_bitrate_max ||
> !i_bitrate_max) ) ||
> - ( hdr->stream[i].i_bitrate <
> hdr->stream[i_video].i_bitrate &&
> - i_bitrate_max != 0 && i_bitrate_total >
> i_bitrate_max )
> - ) ) ) )
> + /* video */
> + else if( hdr->stream[i].i_cat == ASF_CODEC_TYPE_VIDEO &&
> b_video && ( hdr->stream[i].i_bitrate < i_bitrate_max ||
> !i_bitrate_max ) )
> {
> - /* unselect old stream */
> - if( i_video > 0 )
> - {
> - hdr->stream[i_video].i_selected = 0;
> - if( hdr->stream[i_video].i_bitrate> 0 )
> - {
> - i_bitrate_total -= hdr->stream[i_video].i_bitrate;
> - }
> - }
> + courant_CandidateVideo =
> asf_AddCandidate(courant_CandidateVideo, i);
> + }
> + }
> +
> + if( head_Candidatefavlang->is_next )
> + {
> + work_Candidate = head_Candidatefavlang;
> + }
> + else
> + {
> + work_Candidate = head_Candidateotherlang;
> + }
> +
> + courant_CandidateVideo = head_CandidateVideo;
>
> - hdr->stream[i].i_selected = 1;
> - if( hdr->stream[i].i_bitrate> 0 )
> + while( work_Candidate->is_next )
> + {
> + while(courant_CandidateVideo->is_next )
> + {
> + i_bitrate_var =
> hdr->stream[work_Candidate->value].i_bitrate +
> hdr->stream[courant_CandidateVideo->value].i_bitrate;
> + if(i_bitrate_var > i_bitrate_total && ( !i_bitrate_max
> || i_bitrate_var < i_bitrate_max ) )
> {
> - i_bitrate_total += hdr->stream[i].i_bitrate;
> + i_audio = work_Candidate->value;
> + i_video = courant_CandidateVideo->value;
> + i_bitrate_total = i_bitrate_var;
> }
> - i_video = i;
> + courant_CandidateVideo = courant_CandidateVideo->next;
> }
>
> + i_bitrate_var = hdr->stream[work_Candidate->value].i_bitrate;
> + if(i_bitrate_var > i_bitrate_total && ( !i_bitrate_max ||
> i_bitrate_var < i_bitrate_max ) )
> + {
> + i_audio = work_Candidate->value;
> + i_bitrate_total = i_bitrate_var;
> + }
> + work_Candidate = work_Candidate->next;
> }
> +
> + /* select stream */
> + if( i_audio != -1 ) hdr->stream[i_audio].i_selected = 1;
> + if( i_video != -1 ) hdr->stream[i_video].i_selected = 1;
> +
> + /* clear structures */
> + asf_ClearCandidate( head_Candidatefavlang );
> + asf_ClearCandidate( head_Candidateotherlang );
> + asf_ClearCandidate( head_CandidateVideo );
> }
> diff --git a/modules/access/mms/asf.h b/modules/access/mms/asf.h
> index f15b90d..d13aacd 100644
> --- a/modules/access/mms/asf.h
> +++ b/modules/access/mms/asf.h
> @@ -42,6 +42,7 @@ typedef struct
> int i_cat; /* ASF_CODEC_TYPE_VIDEO, ASF_CODEC_TYPE_AUDIO, */
> int i_bitrate; /* -1 if unknown */
> int i_selected;
> + int i_lang_id;
> } asf_stream_t;
>
> typedef struct
> @@ -49,16 +50,25 @@ typedef struct
> int64_t i_file_size;
> int64_t i_data_packets_count;
> int32_t i_min_data_packet_size;
> + char * tablang[128];
>
> asf_stream_t stream[128];
>
> } asf_header_t;
>
> +typedef struct asf_candidate_t
> +{
> + int value ;
> + bool is_next ;
> + struct asf_candidate_t *next ;
> +} asf_candidate_t ;
Do you really need the is_next part? Can't you just check if next is
NULL ?
> - var_InheritBool( p_access, "video" ) );
> + var_InheritBool( p_access, "video" ),
> + var_InheritString( p_access->p_parent,
> "audio-language" ) );
This memleaks, IIRC.
> @@ -718,7 +719,8 @@ static int Describe( access_t *p_access, char
> **ppsz_location )
> var_InheritInteger( p_access, "mms-maxbitrate" ),
> var_InheritBool( p_access, "mms-all" ),
> var_InheritBool( p_access, "audio" ),
> - var_InheritBool( p_access, "video" ) );
> + var_InheritBool( p_access, "video" ),
> + var_InheritString( p_access->p_parent,
> "audio-language" ) );
> return VLC_SUCCESS;
idem.
> diff --git a/modules/access/mms/mmstu.c b/modules/access/mms/mmstu.c
> index 932c59f..5ac3355 100644
> --- a/modules/access/mms/mmstu.c
> +++ b/modules/access/mms/mmstu.c
> @@ -792,7 +792,8 @@ static int MMSOpen( access_t *p_access,
> vlc_url_t *p_url, int i_proto )
> var_InheritInteger( p_access,
> "mms-maxbitrate" ),
> var_InheritBool( p_access, "mms-all" ),
> var_InheritBool( p_access, "audio" ),
> - var_InheritBool( p_access, "video" ) );
> + var_InheritBool( p_access, "video" ),
> + var_InheritString( p_access->p_parent,
> "audio-language" ) );
Idem.
The rest is fine,
Best regards,
--
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device
More information about the vlc-devel
mailing list