From: Stefan H. <ma...@s-...> - 2003-10-09 23:18:02
Attachments:
xine_rewire_bug.c
|
Hi! the attached code demonstrates several bugs i found: - jumpy video playback if a visualisation is used with a/v streams (lot of video_out and metronom log) - xine_post_wire_audio_port hangs until end of stream - xine api: xine_post_output returns const pointer but xine_post_wire_audio_port want a non-const ptr (compiler warning) if my code is buggy, please let me know ;) -- RY Stefan +-----------------+----------------+ | ma...@s-... | www.s-holst.de | +-----------------+----------------+ |
From: Michael R. <mr...@us...> - 2003-10-10 10:14:19
|
Hi Stefan, > the attached code demonstrates several bugs i found: Nice testing! > - jumpy video playback if a visualisation is used with > a/v streams (lot of video_out and metronom log) Could you attach that? > - xine_post_wire_audio_port hangs until end of stream Could you send a backtrace of that deadlock? > - xine api: xine_post_output returns const pointer but > xine_post_wire_audio_port want a non-const ptr (compiler > warning) Whoops, "const" bugging me again. Would it be an incompatible API change, if I removed the "const" from xine_post_output()? > if my code is buggy, please let me know ;) I don't think so. Michael -- panic("esp: what could it be... I wonder..."); 2.2.16 /usr/src/linux/drivers/scsi/esp.c |
From: Stefan H. <ma...@s-...> - 2003-10-10 10:34:09
|
On 10/10, Michael Roitzsch wrote: > Hi Stefan, > > > the attached code demonstrates several bugs i found: > > Nice testing! > > > - jumpy video playback if a visualisation is used with > > a/v streams (lot of video_out and metronom log) > > Could you attach that? yep > > - xine_post_wire_audio_port hangs until end of stream > > Could you send a backtrace of that deadlock? attached i think it's thread 1: #4 0x4003d88d in fifo_wait_empty (fifo=0x40182980) at audio_out.c:385 -- RY Stefan +-----------------+----------------+ | ma...@s-... | www.s-holst.de | +-----------------+----------------+ |
From: Michael R. <mr...@us...> - 2003-10-10 11:58:52
|
Hi Stefan, > > > - jumpy video playback if a visualisation is used with > > > a/v streams (lot of video_out and metronom log) > > > > Could you attach that? > > yep Problem seen and understood. Both video feeds of the decoder and of goom are passed through the same metronom which tries to mediate between them. Bad idea. It might take a until I can get to this, but there should be an easy solution. > > > - xine_post_wire_audio_port hangs until end of stream > > > > Could you send a backtrace of that deadlock? > > attached > > i think it's thread 1: > #4 0x4003d88d in fifo_wait_empty (fifo=0x40182980) at > audio_out.c:385 I don't fully understand this one right now. Give me some time to finish other things. Or maybe someone else wants to have a look? My understanding: The close of the audio port to be unwired never returns. Michael -- "I heard if you play the NT-4.0-CD backwards, you get a satanic message." "That's nothing, if you play it forward, it installs NT-4.0" |
From: Thibaut M. <tma...@vi...> - 2003-10-10 12:09:15
|
Hi Michael, Surlignage Michael Roitzsch <mr...@us...>: > Hi Stefan, > > > > > - jumpy video playback if a visualisation is used with > > > > a/v streams (lot of video_out and metronom log) > > > > > > Could you attach that? > > > > yep > > Problem seen and understood. Both video feeds of the decoder and of goom > are passed through the same metronom which tries to mediate between > them. Bad idea. It might take a until I can get to this, but there > should be an easy solution. Is there no way to refuse to attach a video post plugin if the stream has video ? Thibaut |
From: Michael R. <mr...@us...> - 2003-10-10 20:55:53
|
Hi Thibaut, > > Problem seen and understood. Both video feeds of the decoder and of > > goom are passed through the same metronom which tries to mediate > > between them. Bad idea. It might take a until I can get to this, > > but there should be an easy solution. > > Is there no way to refuse to attach a video post plugin if the stream > has video ? I think that's too restrictive. We just need to recognize in video_out.c, that these frames should not be passed through metronom (or only partially, without modifying metronom's internal state). Michael -- "When you say "I wrote a program that crashed Windows", people just stare at you blankly and say "Hey, I got those with the system, *for free*." -Linus Torvalds |
From: Michael R. <mr...@us...> - 2003-10-26 18:54:36
Attachments:
xine-post-NULL-stream.patch
|
Hi Stefan, > > > - jumpy video playback if a visualisation is used with > > > a/v streams (lot of video_out and metronom log) > > > > Could you attach that? > > yep I looked into that one, could you try, if the attached patch fixes the issue for you? It modifies the engine, so that the stream member in video frames and audio buffers is allowed to be NULL. This makes sense, because these packets do not really originate from a stream, they are generated by a post plugin. With a NULL stream, they will also not pass through metronom and therefore not clutter the vpts generation. Only problem: I had to change the way the post plugins generate the timestamps for the frames, I am not sure, if the post effect is still in sync with the video, but in theory it should be. If the patch works for you and noone objects, I am going to commit it. > > > - xine_post_wire_audio_port hangs until end of stream > > > > Could you send a backtrace of that deadlock? > > attached > > i think it's thread 1: > #4 0x4003d88d in fifo_wait_empty (fifo=0x40182980) at > audio_out.c:385 I looked through the trace and decided that this is not a xine problem. ;) As you already found out, thread 1 is the one that blocks while it should not. So let's check, what it waits for. It's hanging in fifo_wait_empty() during ao_close(). This should not happen. The audio port involved is 0x80adfd8 (see ao_close()'s parameters). The audio loop of this port is thread 4 (thread 4's ao_loop() has the same 0x80adfd8 as parameter). This loop should eventually draw the last audio buffer from the fifo and issue the empty signal our main thread is waiting for. But it seems to be locked in a write() syscall, so something seems to be wrong with the audio driver here. For your const-problem: I think it should be possible to remove the const from the header without losing API compatibility. If noone objects, I will commit that tomorrow and see, if someone complains. Michael -- /* Thanks to Rob `CmdrTaco' Malda for not influencing this code in any * way. */ 2.4.3 linux/net/core/netfilter.c |
From: Stefan H. <ma...@s-...> - 2003-10-30 19:34:55
|
On 10/26, Michael Roitzsch wrote: > Hi Stefan, > > > > > - jumpy video playback if a visualisation is used with > > > > a/v streams (lot of video_out and metronom log) > > > > > > Could you attach that? > > > > yep > > I looked into that one, could you try, if the attached patch fixes the > issue for you? It modifies the engine, so that the stream member in > video frames and audio buffers is allowed to be NULL. This makes sense, > because these packets do not really originate from a stream, they are > generated by a post plugin. With a NULL stream, they will also not pass > through metronom and therefore not clutter the vpts generation. > > Only problem: I had to change the way the post plugins generate the > timestamps for the frames, I am not sure, if the post effect is still > in sync with the video, but in theory it should be. With this patch, video output works fine but visualisation output only works smoothly if it has the same fps as the video. It's better than before anyway. :-) > I looked through the trace and decided that this is not a xine > problem. ;) > > As you already found out, thread 1 is the one that blocks while it > should not. So let's check, what it waits for. It's hanging in > fifo_wait_empty() during ao_close(). This should not happen. The audio > port involved is 0x80adfd8 (see ao_close()'s parameters). The audio > loop of this port is thread 4 (thread 4's ao_loop() has the same > 0x80adfd8 as parameter). This loop should eventually draw the last > audio buffer from the fifo and issue the empty signal our main thread > is waiting for. But it seems to be locked in a write() syscall, so > something seems to be wrong with the audio driver here. hmm, isn't it possible that xine engine keeps feeding audio driver with frames on shutdown? I have the following behaviour: on player shutdown, xine hangs at "audio_out: no streams left, closing driver" but I have still audio output until end of stream. After end of stream, xine shuts down normally. > For your const-problem: I think it should be possible to remove the > const from the header without losing API compatibility. If noone > objects, I will commit that tomorrow and see, if someone complains. fine, thanks :) -- RY Stefan +-----------------+----------------+ | ma...@s-... | www.s-holst.de | +-----------------+----------------+ |
From: Thibaut M. <tma...@no...> - 2003-10-27 19:53:00
|
Hi, On Sun, 2003-10-26 at 19:50, Michael Roitzsch wrote: > I looked into that one, could you try, if the attached patch fixes the > issue for you? It modifies the engine, so that the stream member in > video frames and audio buffers is allowed to be NULL. This makes sense, > because these packets do not really originate from a stream, they are > generated by a post plugin. With a NULL stream, they will also not pass > through metronom and therefore not clutter the vpts generation. > > Only problem: I had to change the way the post plugins generate the > timestamps for the frames, I am not sure, if the post effect is still > in sync with the video, but in theory it should be. just two comments: - the video frames are tagged with the predicted audio vpts, so it should be in sync +-drift error. - if two frames are generated from the same audio buffer, the two frames will be tagged with the same vpts, i guess the second frame will have a good chance to be discarded by the video loop. cheers, thibaut |
From: Michael R. <mr...@us...> - 2003-10-28 17:19:50
|
Hi Thibaut, > > I looked into that one, could you try, if the attached patch fixes > > the issue for you? It modifies the engine, so that the stream > > member in video frames and audio buffers is allowed to be NULL. > > This makes sense, because these packets do not really originate > > from a stream, they are generated by a post plugin. With a NULL > > stream, they will also not pass through metronom and therefore not > > clutter the vpts generation. > > > > Only problem: I had to change the way the post plugins generate the > > timestamps for the frames, I am not sure, if the post effect is > > still in sync with the video, but in theory it should be. > > just two comments: > - the video frames are tagged with the predicted audio vpts, so it > should be in sync +-drift error. > - if two frames are generated from the same audio buffer, the two > frames will be tagged with the same vpts, i guess the second frame > will have a good chance to be discarded by the video loop. I see. Now I really understand the code I deleted... stupid me. So I should not do this: frame->pts = pts; frame->vpts = this->stream->metronom->audio_vpts; but rather this: if (pts) { frame->pts = pts; frame->vpts = this->stream->metronom->audio_vpts; pts = 0; } else frame->pts = frame->vpts = 0; Btw, shall we commit your metonom patch? I have been using it for some time now and haven't had any problems. Michael -- prom_printf("Detected PenguinPages, getting out of here.\n"); 2.0.38 /usr/src/linux/arch/sparc/mm/srmmu.c |
From: Michael R. <mr...@us...> - 2003-10-28 17:47:47
|
correcting myself, Hi again, > So I should not do this: > > frame->pts = pts; > frame->vpts = this->stream->metronom->audio_vpts; > > but rather this: > > if (pts) { > frame->pts = pts; > frame->vpts = this->stream->metronom->audio_vpts; > pts = 0; > } else > frame->pts = frame->vpts = 0; A stupid idea, of course. How about: if (pts) { vpts = this->stream->metronom->audio_vpts; frame->pts = pts; frame->vpts = vpts; pts = 0; } else { vpts += frame->duration; frame->pts = 0; frame->vpts = vpts; } At least this could be a temporary solution for now. For possibly after 1.0 (but maybe even before, depending on the engine impact), I think there is a more clean approach: Visualization post plugins could have a private metronom instance, which would be fed with the video frames (which have been assigned the pts values from the audio buffers) to calculate their vpts. All the nice metronom features like drift calculation would immediately be there. This would, however, require metronom modification, so that it does not require access to the stream any more. But since metronom doesn't use the stream for important things, I think this is easy to do. Michael -- panic("sun_82072_fd_inb: How did I get here?"); 2.2.16 /usr/src/linux/include/asm-sparc/floppy.h |
From: Thibaut M. <tma...@no...> - 2003-10-29 00:39:39
|
Hi Michael, On Tue, 2003-10-28 at 18:38, Michael Roitzsch wrote: > correcting myself, yes, the first mail was a bit strange ;-) > Hi again, > > > So I should not do this: > > > > frame->pts = pts; > > frame->vpts = this->stream->metronom->audio_vpts; > > > > but rather this: > > > > if (pts) { > > frame->pts = pts; > > frame->vpts = this->stream->metronom->audio_vpts; > > pts = 0; > > } else > > frame->pts = frame->vpts = 0; > > A stupid idea, of course. > > How about: > > if (pts) { > vpts = this->stream->metronom->audio_vpts; > frame->pts = pts; > frame->vpts = vpts; > pts = 0; > } else { > vpts += frame->duration; > frame->pts = 0; > frame->vpts = vpts; > } hmmm, pts is the audio buf->vpts, right ? this pts can be 0, so this algorithm does'nt work because if the audio pts is 0, vpts will not be initialized correctly. i propose something like that: /**************/ int64_t vpts = 0; [...] if (!vpts) { vpts = this->stream->metronom->audio_vpts; frame->pts = pts; frame->vpts = vpts; pts = 0; } else { vpts += frame->duration; frame->pts = 0; frame->vpts = vpts; } /**************/ > At least this could be a temporary solution for now. For possibly after > 1.0 (but maybe even before, depending on the engine impact), I think > there is a more clean approach: > > Visualization post plugins could have a private metronom instance, which > would be fed with the video frames (which have been assigned the pts > values from the audio buffers) to calculate their vpts. All the nice > metronom features like drift calculation would immediately be there. > > This would, however, require metronom modification, so that it does not > require access to the stream any more. But since metronom doesn't use > the stream for important things, I think this is easy to do. i guess it needs to be discussed. > Michael btw: i don't understand the point to activate a viz plugin when the stream has a video part, you get 2 frame producers, the engine is not designed for that. cheers, Thibaut |
From: Michael R. <mr...@us...> - 2003-10-29 19:36:58
|
Hi Thibaut, > yes, the first mail was a bit strange ;-) The second wasn't necessarily better... > > How about: > > > > if (pts) { > > vpts = this->stream->metronom->audio_vpts; > > frame->pts = pts; > > frame->vpts = vpts; > > pts = 0; > > } else { > > vpts += frame->duration; > > frame->pts = 0; > > frame->vpts = vpts; > > } > > hmmm, pts is the audio buf->vpts, right ? Yes. Before passing metronom, this member holds pts, not vpts. > this pts can be 0, so this algorithm does'nt work because if the > audio pts is 0, vpts will not be initialized correctly. Sigh. I forgot that. > i propose something like that: > > /**************/ > int64_t vpts = 0; > [...] > if (!vpts) { > vpts = this->stream->metronom->audio_vpts; > frame->pts = pts; > frame->vpts = vpts; > pts = 0; > } else { > vpts += frame->duration; > frame->pts = 0; > frame->vpts = vpts; > } > /**************/ Great. Thanks for your help. I will commit things tomorrow. > > At least this could be a temporary solution for now. For possibly > > after 1.0 (but maybe even before, depending on the engine impact), > > I think there is a more clean approach: > > > > Visualization post plugins could have a private metronom instance, > > which would be fed with the video frames (which have been assigned > > the pts values from the audio buffers) to calculate their vpts. All > > the nice metronom features like drift calculation would immediately > > be there. > > > > This would, however, require metronom modification, so that it does > > not require access to the stream any more. But since metronom > > doesn't use the stream for important things, I think this is easy > > to do. > > i guess it needs to be discussed. I will keep that idea for later. > btw: i don't understand the point to activate a viz plugin when the > stream has a video part, you get 2 frame producers, the engine is not > designed for that. Why not? Opening two video ports and assigning the stream's video to the first and the visualized audio to the second is supposed to work. At least I intended things like that with the post plugin scheme. More elaborate wirings might pose additional problems, but I think the engine can be tweaked to do such things. Michael -- "The nice thing about standards is that there are so many of them to choose from." -Andrew Tanenbaum |
From: Michael R. <mr...@us...> - 2003-10-30 22:40:57
|
Hi Stefan, > With this patch, video output works fine but visualisation output > only works smoothly if it has the same fps as the video. It's better > than before anyway. :-) There might be some XLock or other scheduling problems involved. The two video outputs are completely independent, so they should no affect one another. I committed the patch to cvs. > > As you already found out, thread 1 is the one that blocks while it > > should not. So let's check, what it waits for. It's hanging in > > fifo_wait_empty() during ao_close(). This should not happen. The > > audio port involved is 0x80adfd8 (see ao_close()'s parameters). The > > audio loop of this port is thread 4 (thread 4's ao_loop() has the > > same 0x80adfd8 as parameter). This loop should eventually draw the > > last audio buffer from the fifo and issue the empty signal our main > > thread is waiting for. But it seems to be locked in a write() > > syscall, so something seems to be wrong with the audio driver here. > > hmm, isn't it possible that xine engine keeps feeding audio driver > with frames on shutdown? I have the following behaviour: on player > shutdown, xine hangs at "audio_out: no streams left, closing driver" > but I have still audio output until end of stream. After end of > stream, xine shuts down normally. Is this with your rewiring test program or with normal xine-ui? Michael -- "Hey, it's Unix! I know this!" -Lex, Jurassic park. |
From: Stefan H. <ma...@s-...> - 2003-10-30 23:38:08
|
Hi Michael, On 10/30, Michael Roitzsch wrote: > > hmm, isn't it possible that xine engine keeps feeding audio driver > > with frames on shutdown? I have the following behaviour: on player > > shutdown, xine hangs at "audio_out: no streams left, closing driver" > > but I have still audio output until end of stream. After end of > > stream, xine shuts down normally. > > Is this with your rewiring test program or with normal xine-ui? it is with xiron (sf.net/projects/xiron), a front end framework i'm currently working on. but i can also hack a small test program if you want. it only occurs, if i use visualisations with video streams. -- RY Stefan +-----------------+----------------+ | ma...@s-... | www.s-holst.de | +-----------------+----------------+ |
From: Michael R. <mr...@us...> - 2003-10-31 13:17:21
|
Hi Stefan, > > > hmm, isn't it possible that xine engine keeps feeding audio > > > driver with frames on shutdown? I have the following behaviour: > > > on player shutdown, xine hangs at "audio_out: no streams left, > > > closing driver" but I have still audio output until end of > > > stream. After end of stream, xine shuts down normally. > > > > Is this with your rewiring test program or with normal xine-ui? > > it is with xiron (sf.net/projects/xiron), a front end framework i'm > currently working on. but i can also hack a small test program if you > want. it only occurs, if i use visualisations with video streams. Another one of those nice little test programs would be appreciated. :) Michael -- /* Nobody will ever see this message :-) */ panic("Cannot initialize video hardware\n"); 2.0.38 /usr/src/linux/arch/m68k/atari/atafb.c |
From: Stefan H. <ma...@s-...> - 2003-10-31 15:19:57
Attachments:
xine_audio_close_bug.c
|
On 10/31, Michael Roitzsch wrote: > Hi Stefan, > > > > > hmm, isn't it possible that xine engine keeps feeding audio > > > > driver with frames on shutdown? I have the following behaviour: > > > > on player shutdown, xine hangs at "audio_out: no streams left, > > > > closing driver" but I have still audio output until end of > > > > stream. After end of stream, xine shuts down normally. > > > > > > Is this with your rewiring test program or with normal xine-ui? > > > > it is with xiron (sf.net/projects/xiron), a front end framework i'm > > currently working on. but i can also hack a small test program if you > > want. it only occurs, if i use visualisations with video streams. > > Another one of those nice little test programs would be appreciated. :) here is is :) -- RY Stefan +-----------------+----------------+ | ma...@s-... | www.s-holst.de | +-----------------+----------------+ |