Opened 12 years ago

Closed 12 years ago

#406 closed defect (fixed)

Audio support (and content sniffing branch)

Reported by: Christopher Allan Webber Owned by: joar
Priority: major Milestone: 0.3.0
Component: component1 Keywords:
Cc: Parent Tickets:

Description

Joar Wandborg has been working on an audio support / content sniffing and negotiation branch for mediagoblin, so this bug's purpose is to track it!

Change History (5)

comment:1 by Christopher Allan Webber, 12 years ago

Okay, I've reviewed your branch, and here are my following comments:

  • Trying to figure out the state of imports. You have audiolab and numpy... are thoese both for the visualizations?
  • The thumbnail view looks *awesome*. But the "visualization" spectrogram on the media page maybe looks slightly bland in comparison? I wonder if there's a way to do visualization with javascript. That's probably a huge distraction though. I'd be interested in schendje taking a look at it.
  • These argument headers are long. Instead of:
def create_wave_images(input_filename, output_filename_w, output_filename_s, image_width, image_height, fft_size, progress_callback=None):
    pass

you could do:

def create_wave_images(input_filename, output_filename_w, output_filename_s,
                       image_width, image_height, fft_size,
                       progress_callback=None):
    pass
  • Does it end up doing a temporary write/seek on disk of the file even if the media type does sniffing detection on filetype only? Maybe there's a more efficient algorithm, like:
  • Based on media processor information, create a memoized dictionary like:
{"ogg": [
  {"processor": video_processor_callable,
   "sniffer": video_sniffer_func},
  {"processor": audio_processor_callable,
   "sniffer": audio_sniffer_func}],
 "txt": [
  {"processor": asciiart_processor_callable,
   "sniffer": None}],
 "mp4": [
  {"processor": audio_processor_callable,
   "sniffer": audio_sniffer_func}]}
  • Does the extension have supported processors with keys associated with it? If so, do the following:
    • If there's only one processor, accept that one.
    • Iterate through processors. Accept the first one whose sniffer says they support it. "None" is accepted as "yes"... or alternately, sort the ones with sniffers before the ones without.
  • This code makes me nervous:
# This might fail if this module is loaded before the global_config
# is parsed although this far it has not.
THUMB_SIZE = (mgg.global_config['media:thumb']['max_width'],
              mgg.global_config['media:thumb']['max_height'])

Could we just call this in the function instead of using a global
variable? I'd rather things not break on import just because some
setup function hasn't been run!

comment:2 by Christopher Allan Webber, 12 years ago

Also, would be good to have a unit test for the negotiation function!

in reply to:  1 comment:3 by joar, 12 years ago

Status: newassigned

Replying to cwebber:

Okay, I've reviewed your branch, and here are my following comments:

  • Trying to figure out the state of imports. You have audiolab and numpy... are thoese both for the visualizations?
  • The thumbnail view looks *awesome*. But the "visualization" spectrogram on the media page maybe looks slightly bland in comparison? I wonder if there's a way to do visualization with javascript. That's probably a huge distraction though. I'd be interested in schendje taking a look at it.

It's something for schendje to look at. I have been thinking of a line scrolling through the static spectrogram, and perhaps make the spectrogram scroll for longer clips, although that would require some dynamic loading of spectrogram images since we probably don't want to load a huge spectrogram in each request to that page.

The static spectrogram is something of a placeholder.

  • These argument headers are long. Instead of:
def create_wave_images(input_filename, output_filename_w, output_filename_s, image_width, image_height, fft_size, progress_callback=None):
    pass

you could do:

def create_wave_images(input_filename, output_filename_w, output_filename_s,
                       image_width, image_height, fft_size,
                       progress_callback=None):
    pass

That code is from the http://www.assembla.com/code/freesound/git/nodes code, as indicated by the symlink :)

Should I clean it up? Re-write copying it? Leave it as it is? How would we license that? Perhaps just as I did with my "Modified..." addition to the license notice.

  • Does it end up doing a temporary write/seek on disk of the file even if the media type does sniffing detection on filetype only? Maybe there's a more efficient algorithm, like:
  • Based on media processor information, create a memoized dictionary like:
{"ogg": [
  {"processor": video_processor_callable,
   "sniffer": video_sniffer_func},
  {"processor": audio_processor_callable,
   "sniffer": audio_sniffer_func}],
 "txt": [
  {"processor": asciiart_processor_callable,
   "sniffer": None}],
 "mp4": [
  {"processor": audio_processor_callable,
   "sniffer": audio_sniffer_func}]}
  • Does the extension have supported processors with keys associated with it? If so, do the following:
    • If there's only one processor, accept that one.
    • Iterate through processors. Accept the first one whose sniffer says they support it. "None" is accepted as "yes"... or alternately, sort the ones with sniffers before the ones without.

Media sniffing could be more efficient, I'll look into it.

  • This code makes me nervous:
# This might fail if this module is loaded before the global_config
# is parsed although this far it has not.
THUMB_SIZE = (mgg.global_config['media:thumb']['max_width'],
              mgg.global_config['media:thumb']['max_height'])

Could we just call this in the function instead of using a global
variable? I'd rather things not break on import just because some
setup function hasn't been run!

I have never had that break on me, although we could remove those two as I'm pretty sure nothing uses them, we'll see... ;)

comment:4 by joar, 12 years ago

Pushed to audio+sniffing!

Still some points left.

comment:5 by joar, 12 years ago

Resolution: fixed
Status: assignedclosed

Fixed all of it except audioprocessing.py formatting -- A rewrite of audioprocessing is in progress.

Merged into master and pushed!

Note: See TracTickets for help on using tickets.