On Tue, 10 Oct 2006 06:01:47 JST [EMAIL PROTECTED] (Andrew Church) wrote: > > one more problem: should we use a generic pointer? (looks unclean) > > adding flush_video and flush_audio looks even worse. > > One thing I'm hoping we can do once NMS is in place is get rid of > frame_list_t entirely, so this isn't an approach I'd want to take.
Since we're on topic let me explain my thoughts here. First of all, I fully agree that framebuffer handling surely deserve another deeper rewrite. I'd also like to substitute *frame_list_t[1] with something better. But I don't know yet if it's better to have a single generic container, with of course a 'content-type' field, (TCBuffer? TCPacket? TCFrame?) or keep'on going with TCSomethingVideo and TCSomethingAudio (and TCSomethingStuff...) I like slightly more the generic container way, but I'm fine with either ways. And let me guess, I thought you like more the specific container way :) > >- adding encode_last_frame() or something like it. Definitively > > a corner case (how many encoders needs this?), that's the way > > I like less. > > The real problem is that audio codecs (except PCM, of course) don't > support an arbitrary number of samples per data block, and I suspect all > audio encoders will have to deal with this in one way or another. I'm of the same opinion based on codec that I've seen (libavcodec and lame mostly) but I've only suspicious that's true for all audio codecs. > We could > always say that any partial audio frames at the end of the stream get > discarded, and if you want video-frame-accurate audio use PCM, but I don't > think that would go over too well. ;) So I'd say there's justification for > adding an operation like this. Of course I agree on this given that premises, I only wonder if we need a separate method or if it's enough to have a special meaning for existing audio encode operation. > As to the implementation, I'm still wavering between adding a new > method and overlaying encode_audio(). On one hand, I agree with you in > wanting to keep the number of methods down to a minimum; on the other hand, > there's no real clean way to do it within encode_audio(). Let's make another little mockup, a new method will look like int (*encode_audio_final)(TCModuleInstance *self, aframe_list_t *outframe); right? (we can of course replace _final with _finalize or something like it) > Actually, on second thought, we could pass a normal aframe_list_t with > an audio_len of zero, and then codecs that didn't have to flush could just > handle it like an ordinary audio frame. So the last step of encoding > (after the encoder runs out of input data) would look something like: > > aframe_list_t dummy_in, flush_out; // pretend they're initted > dummy_in.audio_len = 0; > encode_audio(&dummy_in, &flush_out); > if (flush_out.audio_len > 0) > multiplex(NULL, &flush_out); // no video, just the flushed audio > > How does that look to you? Looks quite clean and nice to me. I've no objections at all to this. > >- modify close() to return buffered data lead to problems like > > flush() ones (generic pointer? vframe_list_t? aframe_list_t?), > > so doesn't look good. > > That reminds me, did we ever figure out what we were going to do with > open() and close()? Well, I guess I must present my apologies here :) I was supposed to add such methods a while ago but multiple problems preventing me in doing this. I'm really sorry. The only drawback that I see is handling of multiple multiplexors (!!). If a multiplexor instance can handle a single file at once, we must be able to distinguish between a "real" multiplexor (avi, mpeg, mkv, ogg... Last three in long-term vision :) ) and two "fake" multiplexor coupled (raw+raw, raw+wav, yuv4mpeg+wav and combinations...) Maybe a thin abstraction layer could help here. > I took a quick look at tcmodule*.h, but it doesn't > look like they're in the structures yet. Actually, since we already have a > stop(), it might be simpler to let stop() handle file closing and add a > start() method for file opening--that way we can also redefine configure() > so that it's only for configuring and doesn't act like a start() as well. I'm mostly fine with this solution too but I think using open/close is clearer here. Of course configure() must be updated in either way. Best regards, -- Francesco Romani - Ikitt ['people always complain, no matther what you do'] IM contact : (email first, Antispam default deny!) icq://27-83-87-867 known bugs : http://www.transcoding.org/cgi-bin/transcode?Bug_Showcase tiny homepage : http://fromani.exit1.org (see IDEAS if you want send code!)
