GNOME Bugzilla – Bug 661137
Add support for Commodore tapes
Last modified: 2018-05-05 15:18:10 UTC
Created attachment 198481 [details] [review] adds libgsttap, libgsttapenc and libgsttapdec to gst-plugins-bad Here are 2 patches which add support for Commodore tape dumps. They support an efficient way to store tapes used by Commodore computers in the 80s to load and save data. TAP files are used by emulators of Commodore computers (e.g. VICE, CCS64, Hoxs64), so that those emulators can emulate loading programs from tapes. The first patch adds 3 plug-ins to gst-plugins-bad. It is against tag RELEASE-0.10.21 libgsttap: gsttapfiledec: reads TAP files. Sinks x-tap-tap and sources x-tap gsttapfileenc: writes TAP files. Sinks x-tap and sources x-tap-tap gstdmpdec: reads DMP files. Sinks x-tap-dmp and sources x-tap gsttapconvert: changes rate of a TAP stream. Sinks and sources x-tap No external dependencies libgsttapdec: gsttapdec: converts a TAP stream to a sound. Sinks x-tap and sources x-raw-int Depends on external library libtapdecoder.so, an LGPL library downloadable at http://sourceforge.net/projects/wav-prg/files/audiotap/1.6/libtap-1.6-source.zip/download libgsttapenc: gsttapdec: converts a sound to a TAP stream. Sinks x-raw-int and sources x-tap Depends on external library libtapencoder.so, an LGPL library downloadable at http://sourceforge.net/projects/wav-prg/files/audiotap/1.6/libtap-1.6-source.zip/download If libtapdecoder/libtapencoder libraries and headers are not in the standard locations (e.g. headers in /usr/include and libraries in /usr/lib), one can pass --with-libtap-includes and --with-libtap-libs to configure MIME types: x-tap: sequence of pulses recorded on a tape. The duration of each pulse is represented by an unsigned 32-bit number (in machine endianness) x-tap-tap: the TAP container, devised by Per Håkan Sundell and documented at http://www.computerbrains.com/tapformat.html . The extensions proposed at http://groups.google.com/group/comp.emulators.cbm/msg/1e8284ff8d07302f are also supported. The TAP format is supported by all major emulators of Commodore computers. x-tap-dmp: the DMP container, documented at http://www.luigidifraia.com/c64/dc2n/tech.html . Since such format has little support, the main purpose of this is to allow conversion from this format to the much more widespread TAP format. Also, the format is not completely open: "DMP files must only be written using a hard real-time system used for doing the sampling" according to the author. Therefore, the DMP encoder is not included in the patch. Only version 0 supported The second patch add TAP and DMP to gsttypefind, so TAP and DMP files are supported by decodebin(2), and can, for example, be played by Totem.
Created attachment 198482 [details] [review] Add TAP and DMP files to gsttypefind
could you provide patches for 1.0?
Created attachment 255721 [details] [review] Add libgsttap, libgsttapenc and libgsttapdec to gst-plugins-bad Here's a patch that applies to git tag 1.0.6 (that's the version that Ubuntu 13.04 uses)
Comment on attachment 198482 [details] [review] Add TAP and DMP files to gsttypefind Please provide a patch that applies cleanly to 1.0, also please provide it in "git format-patch" format.
Review of attachment 255721 [details] [review]: The comments for the decoder almost all apply to the encoder too. Also run all the .c files through gst-indent :) ::: configure.ac @@ +2040,3 @@ + AC_CHECK_HEADER(tapencoder.h, + [ + AC_CHECK_LIB(tapencoder,tapenc_get_pulse,HAVE_TAPENC=yes) They don't provide pkg-config files for this? Anyway, please put both elements into the same plugin :) ::: ext/tapdec/gsttapdec.c @@ +2,3 @@ + * GStreamer + * Copyright (C) 2005 Thomas Vander Stichele <thomas@apestaart.org> + * Copyright (C) 2005 Ronald S. Bultje <rbultje@ronald.bitfreak.net> Those two never touched this element, so probably remove them :) @@ +85,3 @@ +struct _GstTapDec +{ + GstElement element; Please let this use GstAudioDecoder as base class @@ +123,3 @@ + GST_PAD_ALWAYS, + GST_STATIC_CAPS ("audio/x-raw, " + "format=S32LE, " I assume this is little endian on little endian systems, and big endian on big endian? In that case you should use the GST_AUDIO_NE macro @@ +229,3 @@ + TRUE, G_PARAM_READWRITE); + obj_properties[PROP_WAVEFORM] = + g_param_spec_uint ("waveform", "Waveform", "0=square, 1=triangle, 2=sine (if tapdecoder does not support sine, will fall back to square)", 0, 2, Make this a GEnum and use g_param_spec_enum() here @@ +230,3 @@ + obj_properties[PROP_WAVEFORM] = + g_param_spec_uint ("waveform", "Waveform", "0=square, 1=triangle, 2=sine (if tapdecoder does not support sine, will fall back to square)", 0, 2, + 0, G_PARAM_READWRITE); | G_PARAM_STATIC_STRING everywhere @@ +379,3 @@ + g_value_init (&default_value, G_PARAM_SPEC_VALUE_TYPE(obj_properties[i])); + g_param_value_set_default (obj_properties[i], &default_value); + g_object_set_property (G_OBJECT(filter), g_param_spec_get_name (obj_properties[i]), &default_value); Instead of this you could also just mark the properties as CONSTRUCT @@ +404,3 @@ + "LGPL", + PACKAGE, + "http://wav-prg.sourceforge.net/" Please use the values here as in other plugins ::: ext/tapenc/gsttapenc.c @@ +86,3 @@ +struct _GstTapEnc +{ + GstElement element; Have this use GstAudioEncoder as base class ::: gst/tap/gstdmpdec.c @@ +193,3 @@ + srccaps = gst_caps_copy (gst_pad_template_get_caps (gst_pad_get_pad_template (filter->srcpad))); + srcstructure = gst_caps_get_structure (srccaps, 0); + gst_structure_set_value (srcstructure, "rate", &rate); Just use GstAudioInfo and gst_audio_info_to_caps() here :) @@ +233,3 @@ + out_pulse += inpulse; + if (inpulse > filter->overflow) + ret = GST_FLOW_ERROR; Just error out immediately here? Then you don't need ret2 :) @@ +244,3 @@ + gst_adapter_unmap (filter->adapter); + gst_buffer_resize(newbuf, 0, pos_outpulse * sizeof(guint)); + ret2 = gst_pad_push (filter->srcpad, newbuf); Properly set timestamp and duration on the buffer, based on what you get as input. @@ +271,3 @@ + gst_element_add_pad (GST_ELEMENT (filter), filter->srcpad); + + filter->adapter = gst_adapter_new (); You need to free the adapter in GstObject::finalize. Also you need to implement GstElement::change_state to properly reset when going from PAUSED to READY so the element can be re-used. Also you should implement a event handler on the sinkpad to handle flush events properly, and also the segment event. ::: gst/tap/gsttapconvert.c @@ +150,3 @@ +{ + GstTapConvert *filter = GST_TAP_CONVERT (base); + guint buflen = gst_buffer_get_size (outbuf) / sizeof(guint); Use the size of the GstMapInfo below, also don't use guint. But use a properly sized type (e.g. guint32 if you want 32 bit values). @@ +155,3 @@ + GstMapInfo map; + + gst_buffer_map (outbuf, &map, GST_MAP_READ); mapping can fail ::: gst/tap/gsttapfiledec.c @@ +118,3 @@ + GST_PAD_ALWAYS, + GST_STATIC_CAPS ("audio/x-tap, " + "rate = (int) { 110840, 111860, 123156, 127840, 138550 }") You don't use these values anywhere else, are they useful? @@ +319,3 @@ + gst_element_add_pad (GST_ELEMENT (filter), filter->srcpad); + + filter->adapter = gst_adapter_new (); Same as for the dmpdec element, including the handling of events and GstElement::change_state. ::: gst/tap/gsttapfileenc.c @@ +358,3 @@ + gst_segment_init(&segment, GST_FORMAT_BYTES); + gst_pad_push_event (filter->srcpad, + gst_event_new_segment (&segment)); You should check if it worked, and also initially check if you can seek at all. See the seeking query, some muxers are using that
Created attachment 259963 [details] [review] 255721: Add libgsttap and libgsttapcodec to gst-plugins-bad Here is a new version of the patch, which should apply to git master. Almost all of the comments have been addressed. Here's an explanation of which ones haven't been addressed. > They don't provide pkg-config files for this? Unfortunately not > Just use GstAudioInfo and gst_audio_info_to_caps() here :) This decoder does not source audio/x-raw, and gst_audio_info_to_caps() seems to produce only audio/x-raw caps. > Properly set timestamp and duration on the buffer, based on what you get as input. With the new patch, dts and duration are set. I tried setting pts as well, but, in that case, playing the file to an audio sink produced an audible gap at regular intervals ( > and also the segment event I could not find any examples where the segment event is handled, to get inspiration from. change_state is now handled, and tapenc also handles flushes (base audio_encoder sending a null frame) > You don't use these values anywhere else, are they useful? The only possible output rates from this file format are in tap_clocks, a few lines below that. > initially check if you can seek at all. See the seeking query, some muxers are using that I couldn't find any cases where gst_query_new_seeking() is called initially, only as a reaction to GST_QUERY_SEEKING.
Created attachment 260010 [details] [review] Add TAP and DMP files to gsttypefind (applies to gst-plugin-base tag 1.2.0)
Created attachment 260177 [details] [review] Add libgsttap and libgsttapcodec to gst-plugins-bad New version of the patch. Correct type of dmpdec element: it's a parser. Also add container tag to tapfiledec and dmpdec
Thanks for your patches. I pushed the typefinding functions, but we think this a bit too fringe to go in the main GStreamer modules. With the typefinding support (and caps therefore) merged in, you can create your own external module which will be usable by GStreamer.