[MPlayer-dev-eng] [PATCH] matroska chapter seeking

[Nico Sabbi]

Evgeniy Stepanov wrote:

>On Saturday 15 July 2006 03:58, Nico Sabbi wrote:
>  
>
>>#define DEMUXER_CTRL_SEEK_TO_CHAPTER 13
>>
>>then in mplayer.c:
>>
>>int chap = 3;
>>
>>demux_control(demuxer, DEMUXER_CTRL_SEEK_TO_CHAPTER, &chap);
>>
>>
>>and something similar to obtain from the demuxer the list of available
>>chapters
>>(according to an hypotetical chapter_t struct defined in a file
>>accessible to other demuxers,
>>such as demuxer.h)
>>    
>>
>
>If demuxer contains chapters infomation, there is no need for 
>DEMUXER_CTRL_SEEK_TO_CHAPTER, regular seeking mechanism can be used.
>
>2nd try. This time chapter struct is made demuxer-independent, demux_mkv sets 
>demuxer->chapters and demuxer->num_chapters when headers are parsed.
>  
>
>------------------------------------------------------------------------
>
>  
>

this patch is much better, but there a couple of observations:

> 
>@@ -2584,6 +2610,9 @@
>         }
>     }
> 
>+  demuxer->chapters = mkv_d->chapters;
>+  demuxer->num_chapters = mkv_d->num_chapters;
>+
>
>  
>

I really don't like those two lines; instead you should memcpy (or 
assign directly to demuxer)
the chapters structure


>@@ -3809,7 +3845,6 @@
>     }
> }
> 
>-
> demuxer_desc_t demuxer_desc_matroska = {
>   "Matroska demuxer",
>   "mkv",
>  
>

cosmetics
You totally missed the uninit part of demuxer->chapters :)
As for the rest I don't have any objection