GNOME Bugzilla – Bug 340492
[flacdec] support push-based operation (and thus flac-over-DAAP)
Last modified: 2006-06-20 19:42:39 UTC
Hi, currently flacdec doesn't support push mode. This prevents streaming of flac over http as gnomevfssrc doesn't support pull mode over http and is the only plugin supporting http for now (neonhttpsrc beeing in -bad... does this support pull?). This for example prevents you from playing flac files over daap with banshee: "Playback Error GstFlacDec cannont work in push mode. The operation is not supported with this source element or protocol" Bye Ubuntu Bug: https://launchpad.net/distros/ubuntu/+source/gst-plugins-good0.10/+bug/42689
*** Bug 340698 has been marked as a duplicate of this bug. ***
"I have FLACs and MP3s on my computer. I have Rhythmbox open. I open Rhythmbox on another computer and it can't play any of the FLACs, only the MP3s." Should this really be marked "enhancement"? Perhaps I don't know the internals, but from an end user perspective, this just isn't working as it "should".
It's just semantics, it doesn't really make a difference whether you mark it as enhancement or not. Arguably, the absence of something working or the absence of a feature is always a 'bug' from the user perspective. For us, however, making something work in streaming mode that only works with random access so far is an enhancement :) FWIW, I have started working on this, but it's not finished yet (and I'm moving house tomorrow so it's going to take a bit before this gets committed).
OK, cool; thanks for the clarification. And I just got done moving, so good luck! Are there any workarounds in the meantime? I have mostly FLACs on my desktop and not being able to listen to them on my laptop (via RB/Banshee or mt-daapd) has been rather frustrating. I feel rather miffed at the various levels of FLAC support with general programs and portable devices; maybe I need to file an enhancement on $world? :P
> Are there any workarounds in the meantime? No work-arounds that I know of besides exporting and mounting your entire Music directory via NFS/samba (so that it appears as a local file to rhythmbox and the kernel takes care of the rest) I'm afraid.
Created attachment 67168 [details] [review] preliminary patch implementing push-mode in flacdec this works, and streams flac over daap successfully. not sure I've caught all the corner cases yet. Review appreciated.
> this works, and streams flac over daap successfully. not sure I've caught all > the corner cases yet. Review appreciated. Haven't tried it with DAAP, but the patch doesn't really work too well for me in other (unframed) chain-based mode, like in a simple gst-launch-0.10 filesrc location=foo.flac ! queue ! flacdec ! fakesink This is mostly due the somewhat odd way the libflac stream decoder operates, I think. Your patch is in essence very similar to what I had originally, only our chain function logic is a bit different. The problem I ran into is this: there doesn't seem to be a way to tell the flac decoder 'I currently don't have more data, let's stop for now and I'll give it to you later'. If the flac stream decoder doesn't get as much data as it wants it will call the read call back again and again until you either error out (which affects the decoder state) or give it all the data it wants. So in my case above the decoder will (at least in my combination of source file/library version) ask for ~64kB of data after it's gone through the headers. In your chain function logic, if there isn't that much data in the adapter, you bail out with ABORT and then call Flac__stream_decoder_flush(), which effectively discards all bytes the decoder has already read, no? In any case, what seems to work for me is to only call Flac__stream_decoder_process_single() if we have at least FLAC__MAX_BLOCK_SIZE bytes in the adapter. Anyway, I've merged your patch into mine (you had some things I forgot about, like the DISCONT buffer flag handling and did some things nicer) and will attach it so you can test it with DAAP. (btw, the void * trick for the callbacks unfortunately didn't work for me - my compiler still complains about wrong callback signatures with those, hence the ugly wrappers).
Created attachment 67533 [details] [review] new patch New patch, please test with DAAP. Works for me with framed and unframed input, the latter both from file and over http.
Weird - I'm surprised my patch didn't work in the unframed case - since that was the main one I used to test it. On the other hand, I have exactly 2 FLAC files I was testing with (the ones from the media test files). The trick of returning ABORT from the read callback and then flushing to reset the decoder state seemed to work ok here. Either way, your patch looks better to me. I'll test it now and then you can commit.
Committed: 2006-06-20 Tim-Philipp Müller <tim at centricular dot net> * ext/flac/Makefile.am: * ext/flac/gstflacdec.c: (gst_flac_dec_init), (gst_flac_dec_reset_decoders), (gst_flac_dec_setup_seekable_decoder), (gst_flac_dec_setup_stream_decoder), (gst_flac_dec_finalize), (gst_flac_dec_metadata_callback), (gst_flac_dec_metadata_callback_seekable), (gst_flac_dec_metadata_callback_stream), (gst_flac_dec_error_callback), (gst_flac_dec_error_callback_seekable), (gst_flac_dec_error_callback_stream), (gst_flac_dec_read_seekable), (gst_flac_dec_read_stream), (gst_flac_dec_write), (gst_flac_dec_write_seekable), (gst_flac_dec_write_stream), (gst_flac_dec_loop), (gst_flac_dec_sink_event), (gst_flac_dec_chain), (gst_flac_dec_convert_sink), (gst_flac_dec_get_sink_query_types), (gst_flac_dec_sink_query), (gst_flac_dec_get_src_query_types), (gst_flac_dec_src_query), (gst_flac_dec_handle_seek_event), (gst_flac_dec_sink_activate), (gst_flac_dec_sink_activate_push), (gst_flac_dec_sink_activate_pull), (gst_flac_dec_change_state): * ext/flac/gstflacdec.h: Support chain-based operation, should make flac-over-DAAP work (#340492). (and apologies for the delay).