Video processing fails when media contains no tags

[ayleph]

The skip_transcode code in mediagoblin/media_types/video/util.py fails with an error if a file does not have metadata tags.

/path/to/mediagoblin/mediagoblin/media_types/video/transcoders.py:34: PyGIWarning: GstPbutils was imported without specifying a version first. Use gi.require_version('GstPbutils', '1.0') before import to ensure that the right version gets loaded.
  from gi.repository import GObject, Gst, GstPbutils
DEBUG:mediagoblin.processing.task:Processing <MediaEntry 7016: Cartel para adopción>
ERROR:mediagoblin.processing.task:An unhandled exception was raised while processing <MediaEntry 7016: Cartel para adopción>
WARNING:mediagoblin.processing:No idea what happened here, but it failed: AttributeError("'NoneType' object has no attribute 'get_string'",)
WARNING:mediagoblin.processing:No idea what happened here, but it failed: AttributeError("'NoneType' object has no attribute 'get_string'",)
Traceback (most recent call last):
  File "bin/gmg", line 9, in <module>
    load_entry_point('mediagoblin', 'console_scripts', 'gmg')()
  File "/path/to/mediagoblin/mediagoblin/gmg_commands/__init__.py", line 142, in main_cli
    args.func(args)
  File "/path/to/mediagoblin/mediagoblin/gmg_commands/reprocess.py", line 293, in reprocess
    run(args)
  File "/path/to/mediagoblin/mediagoblin/gmg_commands/reprocess.py", line 205, in run
    reprocess_info=reprocess_request)
  File "/path/to/mediagoblin/mediagoblin/submit/lib.py", line 261, in run_process_media
    task_id=entry.queued_task_id)
  File "/path/to/mediagoblin/lib/python2.7/site-packages/celery-3.1.17-py2.7.egg/celery/app/task.py", line 547, in apply_async
    link=link, link_error=link_error, **options)
  File "/path/to/mediagoblin/lib/python2.7/site-packages/celery-3.1.17-py2.7.egg/celery/app/task.py", line 739, in apply
    request=request, propagate=throw)
  File "/path/to/mediagoblin/lib/python2.7/site-packages/celery-3.1.17-py2.7.egg/celery/app/trace.py", line 355, in eager_trace_task
    uuid, args, kwargs, request)
  File "/path/to/mediagoblin/lib/python2.7/site-packages/celery-3.1.17-py2.7.egg/celery/app/trace.py", line 253, in trace_task
    I, R, state, retval = on_error(task_request, exc, uuid)
  File "/path/to/mediagoblin/lib/python2.7/site-packages/celery-3.1.17-py2.7.egg/celery/app/trace.py", line 240, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/path/to/mediagoblin/mediagoblin/processing/task.py", line 101, in run
    processor.process(**reprocess_info)
  File "/path/to/mediagoblin/mediagoblin/media_types/video/processing.py", line 396, in process
    vp8_threads=vp8_threads, vorbis_quality=vorbis_quality)
  File "/path/to/mediagoblin/mediagoblin/media_types/video/processing.py", line 265, in transcode
    if skip_transcode(metadata, medium_size):
  File "/path/to/mediagoblin/mediagoblin/media_types/video/util.py", line 37, in skip_transcode
    if config['mime_types'] and tags.get_string('mimetype')[0]:
AttributeError: 'NoneType' object has no attribute 'get_string'

Here's a proposed fix. But I guess the question is, is it valid to have video with tag metadata? If not, then something else may be wrong (bad video file, or perhaps the wrong media type is trying to process the file).

diff --git a/mediagoblin/media_types/video/util.py b/mediagoblin/media_types/video/util.py
index d3d2927..e35b021 100644
--- a/mediagoblin/media_types/video/util.py
+++ b/mediagoblin/media_types/video/util.py
@@ -34,6 +34,9 @@ def skip_transcode(metadata, size):

     _log.debug('skip_transcode config: {0}'.format(config))
     tags = metadata.get_tags()
+    if not tags:
+        return False
+
     if config['mime_types'] and tags.get_string('mimetype')[0]:
         if not tags.get_string('mimetype')[1] in config['mime_types']:
             return False

[ayleph]

Encountered another similar issue in the same skip_transcode function where the code tried to compare a string against an empty video tag. Looks like we need to add checks for none in a lot of the code.

--- a/mediagoblin/media_types/video/util.py
+++ b/mediagoblin/media_types/video/util.py
@@ -48,7 +48,10 @@ def skip_transcode(metadata, size):

     if config['video_codecs']:
         for video_info in metadata.get_video_streams():
-            if not (video_info.get_tags().get_string('video-codec')[1] in
+            tags = video_info.get_tags()
+            if not tags:
+                return False
+            if not (tags.get_string('video-codec')[1] in
                     config['video_codecs']):
                 return False

[Boris Bobrov]

I've changed your patch and applied it as fec916d. Thank you for the patch!

[ayleph]

Re-opening because there were other issues the patch didn't address as mentioned in comment 1. We pretty much to riddle checks throughout the skip_transcode function.

[Boris Bobrov]

oooh, right, my bad. Will re-fix.

[Boris Bobrov]

ayleph, what do you thing about the patch? Does it fix the problem for you?

[ayleph]

Replying to breton:

ayleph, what do you thing about the patch? Does it fix the problem for you?

Definitely addresses the problem I was seeing with video_tags being empty on a real video file. It looks like that takes care of the null cases. I can't think of a good way to test whether the dimensions_match code could fail. So I'd say it looks good to push. Thanks!

[Boris Bobrov]

Merged in 339cfb3