[Mjpeg-developer] check chroma type on each frame necessary?

[Steven M. S. <sms@2BSD.COM>]

Hi -

	I've been reading thru the lavtools/* modules with an eye  towards
	enhancing/fixing the YUV support and noticed something which seems
	strange and quite inefficient.

	In lavtools/editlist.c the function el_video_frame_data_format()
	(which is called for *each* frame) the data format is computed via
	a sequence of strcasecmp() calls:

int el_video_frame_data_format(long nframe, EditList *el)
{
   int n;
   const char *comp;

   if(el->video_frames<=0) return DATAFORMAT_MJPG; /* empty editlist, return default */
   if(nframe<0) nframe = 0;
   if(nframe>el->video_frames) nframe = el->video_frames;
   n = N_EL_FILE(el->frame_list[nframe]);
   comp = lav_video_compressor(el->lav_fd[n]);

   if      (strncasecmp(comp,"yv12",4)==0) return DATAFORMAT_YUV420;
   else if (strncasecmp(comp,"yuv2",4)==0) return DATAFORMAT_YUV422;
   else if (strncasecmp(comp,"dv",2)==0) return DATAFORMAT_DV2;
   else if (strncasecmp(comp,"mjp",3)==0 || strncasecmp(comp,"jpeg",4)==0)
       return DATAFORMAT_MJPG;
   else return -1;
}
	
	Oh, and "yuv2" is a PACKED (not PLANAR) format but there's no
	logic present to convert packed to planar for "YUV" files (there is
	frame_YUV422_to_planar() but it is only called for PAL-DV not general
	"YUV" files).

	Wouldn't it be more efficient to determine the DATAFORMAT once when
	the file is opened (when the 'lav_fd' structure is initialized)?  A
	'dataformat' member of lav_fd would be created for this purpose.

	The function then could become simply something like:

int el_video_frame_data_format(long nframe, EditList *el)
{
   int n;

   if(el->video_frames<=0) return DATAFORMAT_MJPG; /* empty editlist, return default */
   if(nframe<0) nframe = 0;
   if(nframe>el->video_frames) nframe = el->video_frames;
   n = N_EL_FILE(el->frame_list[nframe]);

   return(el->lav_fd[n]->dataformat);
}

	This would also consolidate the 'compressor'(fourcc) checking into
	one place.  At the present time there is some chroma space checking
	being done in lav_io.c and also in editlist.c and the list of values 
	checked for is different: in editlist.c "yv12" and "yuv2" (which is
	a PACKED 4:2:2 format) are compared against but in lav_io.c the 
	strings "yuv" and "yv12" are used to set 4:2:0:

	lav_io.c thinks that anything starting with "yuv" is a 4:2:0 dataformat:

   if (strncasecmp(video_comp,"yuv",3)==0 || strncasecmp(video_comp, "yv12",4)==0)
	 lav_fd->chroma = CHROMA420;
	...

	Thus in one place '4:2:0' will be set, but 4:2:2 in the other - UGH!

	All in all the "yuv" support seems to be a mess and might not be 
	operational.  My hunch is that folks only use "mjpg" and "dv" 
	and thus haven't noticed that the "YUV" code probably doesn't work.

	Having written all that have I missed the obvious?  Did I overlook
	a function or program logic flow that enables Quicktime or AVI files 
	containing YUV data to work?

	Cheers,
	Steven Schultz