theora dev - Feb 2010 - Video encoded using libtheora-1.1.1 and libogg-1.1.4 plays too fast...

I''m new to mailing lists so please forgive me if I break any
protocols...
	I''ve been trying to encode video using libtheora-1.1.1 and
libogg-1.1.4 on a Win32 system.  While I did have to change the code in order to
get them to compile, I''m assuming they''re correct.  With that
assumption, the video I make with them plays way too fast.  Either the framerate
is interpreted by players to be way too high or not all the frames are getting
encoded in the stream.  Media reporting software (like MediaInfo) report the
correct framerate from the ogg file.  I will post the test code that
I''m using so people can take a cursory look and see if there are any
obvious mistakes that I''m making.  If anyone actually wants to compile
the code, I can post a version of it that doesn''t use an external image
library, in this case CxImage.	Any help is greatly appreciated.  Thank you!



#include <stdio.h>//#include <conio.h>#include <assert.h>
#include "CxImage\ximage.h"#include
"theora\theoraenc.h"#include "theora\codec.h"

// Rounds to the next multiple of 16int RoundTo16(int in) {	return (in + 15) /
16 * 16;}
// This is a cheap transform but it will do for a simple testunsigned char
TransformPixel(int *tables, RGBQUAD colour) {	return static_cast<unsigned
char>(tables[0] + (tables[1] * colour.rgbRed + tables[2] * colour.rgbGreen +
tables[3] * colour.rgbBlue) / 256);}
// This is done so often that we might as well factor it outvoid
WriteOggPage(FILE *file, ogg_stream_state *state, bool flush = false) {	ogg_page
page;	int rc;	if( flush ) rc = ogg_stream_flush(  state, &page);	else       
rc = ogg_stream_pageout(state, &page);
	if( rc != 0 ) {		fwrite(page.header, 1, page.header_len, file);	
fwrite(page.body, 1, page.body_len, file);	}}
int main(int argc, char *argv[]) {	if( argc < 3 ) {		printf("Too few
parameters...\n");
	} else {		char *input_filename = argv[1];		char *output_filename = argv[2];
		CxImage image;		image.Load(input_filename);
		if( image.IsValid() ) {			printf("Loaded image!\n");
			th_info theora_input;			th_info_init(&theora_input);		
theora_input.frame_width = RoundTo16(image.GetWidth());		
theora_input.frame_height = RoundTo16(image.GetHeight());		
theora_input.pic_width = image.GetWidth();			theora_input.pic_height =
image.GetHeight();			theora_input.pic_x = 0;			theora_input.pic_y = 0;		
theora_input.colorspace = TH_CS_UNSPECIFIED;			theora_input.pixel_fmt =
TH_PF_444;			theora_input.quality = 42;			theora_input.target_bitrate = 0;		
theora_input.fps_numerator = 30;			theora_input.fps_denominator = 1;		
theora_input.aspect_numerator = 0;			theora_input.aspect_denominator = 0;
			th_enc_ctx *theora_state = th_encode_alloc(&theora_input);
			int temp = 5;			//th_encode_ctl(theora_state,
TH_ENCCTL_SET_KEYFRAME_FREQUENCY_FORCE, &temp, sizeof(temp));		
th_encode_ctl(theora_state, TH_ENCCTL_GET_SPLEVEL_MAX, &temp, sizeof(temp));
th_encode_ctl(theora_state, TH_ENCCTL_SET_SPLEVEL    , &temp, sizeof(temp));
			ogg_stream_state ogg_state;			ogg_stream_init(&ogg_state, 3001);    //
Does it really matter what this is?
			FILE* output_file = fopen(output_filename, "wb");			ogg_packet
packet;			bool go = true;
			th_comment comment = {0};			int rc = th_encode_flushheader(theora_state,
&comment, &packet);			ogg_stream_packetin(&ogg_state, &packet); 
// Can this really fail?			WriteOggPage(output_file, &ogg_state, true);
			while(true) {				int rc = th_encode_flushheader(theora_state, &comment,
&packet);				//printf("%i\n", packet.packetno);				if( rc == 0) {	
printf("Done headers!\n");					break;				} else {				
printf("Another packet...\n");				}			
ogg_stream_packetin(&ogg_state, &packet);    // Can this really fail?		
}			WriteOggPage(output_file, &ogg_state, true);
			if( go ) {				printf("Preparing frame...\n");
				th_ycbcr_buffer buffer;				int tables[][4] = {{16, 66, 129, 25}, {128, -38,
-74, 112}, {128, 112, -94, -18}};				for(int i = 0; i < 3; i++) {				
buffer[i].width = theora_input.frame_width;					buffer[i].height =
theora_input.frame_height;					buffer[i].stride = theora_input.frame_width;				
buffer[i].data = new unsigned char[theora_input.frame_width *
theora_input.frame_height];
					for(DWORD y = 0; y < image.GetHeight(); y++) {						for(DWORD x = 0; x
< image.GetWidth(); x++) {							buffer[i].data[y * buffer[i].stride + x] =
TransformPixel(tables[i], image.GetPixelColor(x, image.GetHeight() - y, false));
}					}				}
				printf("Encoding video...\n");
				int num_frames = 30 * 10;    // This should be 10 seconds of video but it
ends up being about 3...				for(int i = 0; i < num_frames; i++) {					int rc
= th_encode_ycbcr_in(theora_state, buffer);					if( rc == 0 ) {						while(true)
{							int rc = th_encode_packetout(theora_state, (i == num_frames - 1) ? 1 :
0, &packet);							if( rc == 0 ) break;						
ogg_stream_packetin(&ogg_state, &packet);						}					
WriteOggPage(output_file, &ogg_state);
					} else {						printf("Encoding error was encountered!\n");					
break;					}				}				WriteOggPage(output_file, &ogg_state, true);
				for(int i = 0; i < 3; i++) {					delete [] buffer[i].data;				}
				fclose(output_file);
				printf("Done encoding!\n");			}
			ogg_stream_clear(&ogg_state);			th_encode_free(theora_state);
		} else {			printf("Bad image input...\n");		}	}
	//_getch();
	return 0;} 		 	   		  
_________________________________________________________________

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://lists.xiph.org/pipermail/theora-dev/attachments/20100211/ec1e499d/attachment.htm

You seem to be assuming a one-to-one correspondence between packets and
pages. This is not correct. A packet can span any number of pages (and
usually does for keyframes), and a page may contain all or part of
multiple packets (as it usually does when you encode the same frame 300
times, as most of these frames will be very small). You need to write
out pages in a loop so long as there are pages ready to be written,
instead of just writing 0 or 1 pages after each packet.

I''m moving this private correspondence to the mailing list since it was
only accidentally made private by me...
> Date: Thu, 11 Feb 2010 03:53:25 -0500
> From: tterribe at email.unc.edu
> 
> Id Kong wrote:
> >>> times, as most of these frames will be very small). You need
to write
> >>> out pages in a loop so long as there are pages ready to be
written,
> >>> instead of just writing 0 or 1 pages after each packet.> 
> 
> > I based my code off of encoder_example.c !  This is why I need you to
be
> > specific.  The problem with the example code is that it''s
enormously
> 
> I thought the above was specific. You need to put WriteOggPage in a loop
> instead of calling it once per frame. This applies to the headers as well.
	I''m sorry, I didn''t read your post carefully enough.  So,
what you''re saying is that not only do th_encode_packetout() and
ogg_stream_packetin() need to be called in a loop but ogg_stream_pageout() needs
to be called in a (different) loop as well, right?	Incidentally,
encoder_example.c does not do this.  If I''m reading the code right,
there is only one call to ogg_stream_pageout() for every loop of
th_encode_packetout() and ogg_stream_packetin() calls.	I tried changing
WriteOggPacket() like so:

void WriteOggPage(FILE *file, ogg_stream_state *state, bool flush = false) {
ogg_page page;	while(true) {		int rc;		if( flush ) rc = ogg_stream_flush( 
state, &page);		else        rc = ogg_stream_pageout(state, &page);
		if( rc == 0 ) {			break;
		} else {			fwrite(page.header, 1, page.header_len, file);			fwrite(page.body,
1, page.body_len, file);		}	}}

	This resulted in a slight larger file but no discernible difference in the
movie play length.  It''s certainly not playing the expected duration.
What do you think?
> > Hey, is this a private e-mail correspondence?  How do I reply to the
group?
> 
> Reply to theora-dev at xiph.org. The list is not configured to do this by
> default (don''t ask me why).
	Okay, so I won''t ask you but someone should have some reason for doing
this.  How often do you want to respond to the group rather than responding
privately to another member of the group?

 		 	   		  
_________________________________________________________________
Introducing Windows? phone.
http://go.microsoft.com/?linkid=9708122
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://lists.xiph.org/pipermail/theora-dev/attachments/20100211/b26a28ee/attachment.htm

Id Kong wrote:
> Incidentally, encoder_example.c does not do this.  If I''m reading
the
> code right, there is only one call to ogg_stream_pageout() for every
> loop of th_encode_packetout() and ogg_stream_packetin() calls.
No, encoder_example is not structured like this at all. It writes pages
one at a time, because it must mux audio and video, and the pages have
to be ordered according to the times associated with their granule
positions. It loops writing one page at a time so long as there is a
page available for both streams, and only if there are no pages
available for one of the streams does it submit more data for
compression (producing more packets).
> This resulted in a slight larger file but no discernible difference in
> the movie play length.  It''s certainly not playing the expected
duration.
> What do you think?
Since you''re just compressing the same frame over and over again, what
you''re likely getting is a keyframe followed by a bunch of zero-byte
packets, which is Theora''s cheap way of saying "keep showing the
last
frame". Some (broken) players treat these as semantically different,
however, and will end playback early when they appear at the end of a
stream. I suggest running ogginfo or oggzinfo on the file and see what
it says the real length is. You may also be interested in oggz-validate,
which can confirm that you are writing out all of the pages in the
proper order.

By the way, please do not use a constant for the stream serial number.
Multiple streams in the same file must have unique serial numbers, and
if someone later wants to concatenate multiple streams you produce, they
will have to rewrite the stream with a new serial number or (more
likely) will generate an invalid file.