[MPlayer-dev-eng] [PATCH] Movie fragments support for demux_mov
Ulion
ulion2002 at gmail.com
Tue Dec 18 08:33:40 CET 2007
2007/12/10, David Romacho <romaxin2002 at yahoo.com.au>:
> David Romacho wrote:
> > Hi all,
> >
> > The goal of these patches is to provide MPlayer with the movie
> > fragment feature present in mov/mp4 container (ISO/IEC 14496-12
> > standard). This feature is used by some digital cameras that save the
> > video in several fragments. Currently MPlayer just recognises the
> > first movie fragment and does not play the rest of fragments.
> >
> > I split the previous patch into 3 different patches. They are supposed
> > to be applied one right after the other against demux_mov.c
> > MPlayer should compile after applying each patch but support for movie
> > fragments won't be fully operational till last patch is applied.
> > So, once all patches are applied, MPlayer should playback the movie
> > fragment test samples (http://samples.mplayerhq.hu/mov/fragments/)
> > completely. The head of MPlayer just plays the first 15 seconds of
> > them (the first movie fragment).
> >
> > Patch description:
> >
> > step1.patch
> >
> > This patch basically adds :
> >
> > * Adds the mov_traf_t struct.
> > * Adds new members to mov_priv_t.
> > * Does not stop parsing when a 'mdat' atom is found. Inserts it
> > into the list and continues.
> > * Recognises 'mfra' atoms in 'mov_check_file' function.
> > * Recognises 'moov' atoms in 'mov_check_file' function.
> >
> > step2.patch
> >
> > This patch adds new functions that parse previously ignored atoms:
> >
> > * lschunks_inmfra: parses 'mfra' atoms. Movie Fragment Random
> > Access Box (for fast seeking). Has 'tfra' and 'mfro' atoms inside.
> > * lschunks_intraf: parses 'traf' atoms. Track Fragment Box. Has
> > 'tfhd' and 'trun' atoms inside.
> > * lschunks_inmoof: parses 'moof' atoms. Movie Fragment Box. Has
> > 'mfhd' and 'traf' atoms inside.
> > * lschunks_inmvex: parses 'mvex' atoms. Movie Extends Box. Has
> > 'mehd' and 'trex' atoms inside.
> >
> > If someone reviews this patch, he will need to take a look to the
> > ISO/IEC 14496-12 Standard documentation.
> > And a couple of new functions that are needed in order to add the traf
> > to each track and finally rebuild indexes.
> >
> > * add_traf_to_track.
> > * mov_rebuild_index.
> >
> > step3.patch
> >
> > * Free allocated memory.
> > * Fix the duration map.
> > * Detect 'mvex' atoms in lschunks function.
> > * Process 'moof' chunks, attach corresponding trafs and rebuild
> > indexes in mov_read_header function.
> >
> >
> > I am also including movie_fragments.patch which includes all the
> > changes. It can can be applied directly to demux_mov.c.
> >
> > Cheers,
> >
> > David
> >
> No one is interested in this feature ? The code is fully functional so
> after applying the patch mplayer is able to reproduce correctly a
> certain kind of MP4/MOV files. No one is encouraged to review these
> patches ?
I can try, review:
+ // If track has keyframes, choose the nearest
+ // (earlier) keyframe. Otherwise, just choose the nearest sample.
+ if(trak->keyframes_size) {
+ unsigned int kfi;
+ for(kfi=0;kfi<trak->keyframes_size-1;kfi++){
+ if(trak->samples[trak->keyframes[kfi+1]].pts > pts){
+ sample = trak->keyframes[kfi];
+ break;
+ }
+ }
Last keyframe is ignored, maybe you should loop from back to front.
+ mp_msg(MSGT_DEMUX,MSGL_V,"MOV: Movie DATA #%"PRIx64"
found!\n",(int64_t)priv->mdat_counter);
+ priv->mdat_start[priv->mdat_counter]=stream_tell(demuxer->stream);
+ priv->mdat_end[priv->mdat_counter]=priv->mdat_start[priv->mdat_counter]+len-skipped;
+ priv->mdat_counter++;
+ mp_msg(MSGT_DEMUX,MSGL_DBG2,"MOV: Movie data #%"PRIx64": start:
%"PRIx64" end: %"PRIx64"\n",
+ (int64_t)priv->mdat_counter,(int64_t)priv->mdat_start[priv->mdat_counter-1],
+ (int64_t)priv->mdat_end[priv->mdat_counter-1]);
mdat_counter is not off_t or int64, need not PRIx64 to print it.
And should keep the indent of the original code (N tabs prefix + M
spaces, 0<=M<7)
And for better alignment with any tab size, the wrapped line should
use the previous line's prefix tab/spaces with additional spaces to
align the two lines to the first parameter of the function call.
But anyway the indent style of the original code is really not good,
it force people use tab size 8 else may got a bad view.
+ //if(flags==3){
// if we're over the headers, then we can stop parsing here!
- demuxer->priv=priv;
- return DEMUXER_TYPE_MOV;
- }
+ //demuxer->priv=priv;
+ //return DEMUXER_TYPE_MOV;
+ //}
use /* */ to comment here will looks better.
+ for (i = 0; i < MOV_MAX_TRAFS; i++) {
+ mov_traf_t *traf = priv->trafs[i];
+ if (traf) {
+ // free all traf fileds here
+ free(traf->samples);
+ free(traf->durmap);
+ free(traf->chunks);
+ }
+ }
ident size should be fixed, and comment not ident correctly.
+ /*be conservative w/this adjustment for the time being. decreasing
+ the framerate and encoding w/o softskip is bound to cause decode
+ errors and possibly cause the output framerate to be suboptimal.
+ really this should be marked as variable framerate in the case
+ of durmap_size>1
+ */
keep cols within 80.
+ if( (trak->samplesize == traf->default_sample_size) ||
traf->default_sample_size==0 ){
+ constant_sample_size = 1;
+ } else {
+ s=0;
+ for(i=0;i<trak->chunks_size;i++){
+ trak->chunks[i].sample=s;
+ s+=trak->chunks[i].size;
+ }
indent fix
+ } else {
+ trak->samples = realloc_struct(trak->samples,
trak->samples_size+traf->samples_size, sizeof(mov_sample_t));
+ for(i=0;i<traf->samples_size;i++)
+ trak->samples[trak->samples_size + i].size = traf->samples[i].size;
+
+ trak->samples_size += traf->samples_size;
+ //mp_msg(MSGT_DEMUX, MSGL_V, "Added %d samples, total
%d\n",traf->samples_size,trak->samples_size);
+ }
same as above.
+ // Add chunks
+ trak->chunks = realloc_struct(trak->chunks,
trak->chunks_size+traf->chunks_size, sizeof(mov_chunk_t));
+ for(i=0;i<traf->chunks_size;i++){
+ trak->chunks[trak->chunks_size + i].size = traf->chunks[i].size;
+ trak->chunks[trak->chunks_size + i].pos = traf->chunks[i].pos;
+ trak->chunks[trak->chunks_size + i].desc = traf->chunks[i].desc;
+ }
Does the chunk struct member sample need be assigned also or just
leave it in undefined value.
Or I suggested use one memcpy to replace the loop.
+ // Add durmaps
+ if( !constant_sample_size ) {
+ trak->durmap = realloc_struct(trak->durmap,
trak->durmap_size+traf->durmap_size, sizeof(mov_durmap_t));
+ for(i=0;i<traf->durmap_size;i++){
+ trak->durmap[trak->durmap_size + i].dur = traf->durmap[i].dur;
+ trak->durmap[trak->durmap_size + i].num = traf->durmap[i].num;
+ }
+ trak->durmap_size += traf->durmap_size;
memcpy.
+ if (trak->samples_size < s) {
+ if( trak->samples_size ) {
+ mp_msg(MSGT_DEMUX, MSGL_WARN,"MOV: durmap or chunkmap
bigger than sample count (%i vs %i)\n",s, trak->samples_size);
+ }
+ trak->samples_size = s;
+ trak->samples = realloc_struct(trak->samples, s, sizeof(mov_sample_t));
+ }
need to set any default value for new allocated part samples?
samples[i].size seem will be used later without assignment.
+ // calc pts:
+ s=0;
+ for(j=0;j<trak->durmap_size;j++){
+ for(i=0;i<trak->durmap[j].num;i++){
+ trak->samples[s].pts=pts;
+ ++s;
+ pts+=trak->durmap[j].dur;
+ if(!trak->durmap[j].dur)
+ pts+=trak->durmap[0].dur;
+ }
+ }
Is it safe to add trak->durmap[0].dur?
+ if( trak->length != pts ) {
+ // normal behavior when TRAFS are present
+ //mp_msg(MSGT_DEMUX, MSGL_WARN,
+ // "MOV: track length is: %d, and durmap length is:
%d\n", trak->length, pts );
+ trak->length = pts;
+ }
indent fix.
+ mp_msg(MSGT_DEMUX,
MSGL_DBG3,"MOV:\t\tlength_size_of_sample_num =
%d\n",length_size_of_sample_num);
+ mp_msg(MSGT_DEMUX, MSGL_DBG3,"MOV:\t\tnumber_of_entry =
%d\n",number_of_entry);
+ int i, j, k, count_trafs;
+ mov_traf_t *traf = NULL;
+ uint64_t count_sample;
declaration not in beginning of scope.
+ }else{
+ time = stream_read_dword(demuxer->stream);;
+ moof_offset = stream_read_dword(demuxer->stream);;
+ }
;;
+ // look for the specified traf
+ if( traf!= NULL ) {
+ if( traf->track_ID == track_ID ) {
+ if( count_trafs == traf_number ) { // already
selected traf, no need to search it
+ if( trun_number<traf->chunks_size &&
trun_number>0 ) {
merge ifs
+ // Alloc arrays
+ if( found_variable_samplesize ) {
+ traf->samples = (mov_sample_t*)
malloc(traf->samples_size*sizeof(mov_sample_t));
+ memset( traf->samples, 0,
traf->samples_size*sizeof(mov_sample_t));
+ } else {
+ traf->samples_size=0;
+ }
+ traf->durmap =
(mov_durmap_t*)malloc(traf->durmap_size*sizeof(mov_durmap_t));
+ traf->chunks = (mov_chunk_t*)malloc(traf->chunks_size*sizeof(mov_chunk_t));
+ memset( traf->durmap, 0, traf->durmap_size*sizeof(mov_durmap_t) );
+ memset( traf->chunks, 0, traf->chunks_size*sizeof(mov_chunk_t));
+
calloc to shorten code.
+ len = 0;
+ pos = start;
+ int sample_count = 0;
+ int durmap_count = 0;
+ int chunk_count = 0;
declaration should not at beginning of scope.
+ priv->trafs[priv->traf_counter] = (mov_traf_t*)
malloc(sizeof(mov_traf_t));
+ memset(priv->trafs[priv->traf_counter],0,sizeof(mov_traf_t));
no force type conversion, and calloc will save one line.
--
Ulion
More information about the MPlayer-dev-eng
mailing list