lisamelton / video_transcoding

Tools to transcode, inspect and convert videos.
MIT License
2.39k stars 160 forks source link

Crash using `--copy-audio-name` if audio track has no name #279

Closed faithfracture closed 5 years ago

faithfracture commented 5 years ago

If you use the --copy-audio-name all option on an MKV that has an audio track with no name transcode-video will crash with {binpath}/transcode-video: undefined method `gsub' for nil:NilClass

khaosx commented 5 years ago

@faithfracture

Thanks for raising this issue! Would you mind providing the entire command line that you used so that we can troubleshoot more effectively? This is likely going to be a @donmelton issue, but the rest of us might catch something if we can see the entire scope.

Thanks!

faithfracture commented 5 years ago

Sure!

This command throws the exception: transcode-video --add-audio all --copy-audio all --copy-audio-name all --audio-width other=double --add-subtitle all --burn-subtitle scan --output transcodedVideoFile.mkv --filter decomb --filter detelecine --crop 0:0:26:8 --dry-run someVideFile.mkv

If I just remove the --copy-audio-name all parameter it runs just fine. Likewise, if I add the name field to the audio track and don't remove the --copy-audio-name all parameter it also runs as expected.

khaosx commented 5 years ago

Thanks! I was hoping we'd see an easy fix, but that command line looks right. Bumping this up to Don.

faithfracture commented 5 years ago

If I had to guess, I'd say it's right here: https://github.com/donmelton/video_transcoding/blob/61f800fbc1e70bcdae9ace2d4c968abb4b834959/lib/video_transcoding/media.rb#L104

lisamelton commented 5 years ago

@faithfracture As @khaosx already wrote, thanks so much for opening this issue. And my apologies for this crash interrupting your use of transcode-video.

When you write, "if I add the name field to the audio track"... Do you mean that you modified your original MKV input file to add a name property using a tool like mkvpropedit?

If so, could you attach the output of a scan of your original unmodified file here using this command?

transcode-video -vv --scan "/path/to/Movie.mkv"

Thanks.

As to the location of the crash, I doubt it would be in the media scanning code since removing --copy-audio-name all would not prevent that code from running. But I've been wrong before. :)

faithfracture commented 5 years ago

Yep. I use the MKVToolNix mac GUI app, but yes - mkvpropedit.

Here's the output from transcode-video

Output ``` $: transcode-video -vv --scan 0001-3483.mkv transcode-video 0.25.1 Copyright (c) 2013-2019 Don Melton Processing: 0001-3483.mkv... Scanning media title 1 with HandBrakeCLI... HandBrake 1.2.2 found... [08:39:32] hb_init: starting libhb thread [08:39:32] thread 7000011e5000 started ("libhb") HandBrake 1.2.2 (2019022400) - Darwin x86_64 - https://handbrake.fr 24 CPUs detected Opening 0001-3483.mkv... [08:39:32] CPU: Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz [08:39:32] - Intel microarchitecture Ivy Bridge [08:39:32] - logical processor count: 24 [08:39:32] hb_scan: path=0001-3483.mkv, title_index=1 udfread ERROR: ECMA 167 Volume Recognition failed disc.c:323: failed opening UDF image 0001-3483.mkv disc.c:424: error opening file BDMV/index.bdmv disc.c:424: error opening file BDMV/BACKUP/index.bdmv [08:39:32] bd: not a bd - trying as a stream/file instead libdvdnav: Using dvdnav version 6.0.0 libdvdread: Encrypted DVD support unavailable. libdvdread:DVDOpenFileUDF:UDFFindFile /VIDEO_TS/VIDEO_TS.IFO failed libdvdread:DVDOpenFileUDF:UDFFindFile /VIDEO_TS/VIDEO_TS.BUP failed libdvdread: Can't open file VIDEO_TS.IFO. libdvdnav: vm: failed to read VIDEO_TS.IFO [08:39:32] dvd: not a dvd - trying as a stream/file instead Input #0, matroska,webm, from '0001-3483.mkv': Metadata: SCENE : Scene FILE : /Volumes/Tantiss/video_transcoding/done/Dirty_Dancing/original/Emile_Ardolino_Q&A-featurette.blend DATE : 2019/05/15 08:32:59 ENCODER : Lavf58.12.100 Duration: 00:01:56.22, start: 0.000000, bitrate: 6023 kb/s Stream #0:0: Video: h264 (High), yuv420p(progressive), 1920x1080 [SAR 1:1 DAR 16:9], 29.97 fps, 29.97 tbr, 1k tbn, 59.94 tbc (default) Metadata: DURATION : 00:01:56.216000000 Stream #0:1: Audio: aac (LC), 48000 Hz, stereo, fltp (default) Metadata: DURATION : 00:01:56.181000000 [08:39:32] scan: decoding previews for title 1 [08:39:32] scan: audio 0x1: aac, rate=48000Hz, bitrate=1 Unknown (AAC LC) (2.0 ch) [08:39:32] scan: 2 previews, 1920x1080, 29.970 fps, autocrop = 0/0/0/0, aspect 16:9, PAR 1:1 [08:39:32] libhb: scan thread found 1 valid title(s) + title 1: + stream: 0001-3483.mkv + duration: 00:01:56 + size: 1920x1080, pixel aspect: 1/1, display aspect: 1.78, 29.970 fps + autocrop: 0/0/0/0 + chapters: + 1: cells 0->0, 0 blocks, duration 00:01:56 + audio tracks: + 1, Unknown (AAC LC) (2.0 ch) (iso639-2: und) + subtitle tracks: HandBrake has exited. {:title=>1, :size=>87500085, :directory=>false, :duration=>116, :width=>1920, :height=>1080, :fps=>29.97, :autocrop=>nil, :audio=>{1=>{:format=>"AAC LC", :channels=>2.0, :language=>"und", :bps=>nil, :stream=>1, :default=>true, :name=>nil}}, :subtitle=>{}, :h264=>true, :hevc=>false, :mpeg2=>false, :stream=>0, :bd=>false, :dvd=>false, :mkv=>true, :mp4=>false} + title 1: + stream: 0001-3483.mkv + duration: 00:01:56 + size: 1920x1080, pixel aspect: 1/1, display aspect: 1.78, 29.970 fps + chapters: + 1: cells 0->0, 0 blocks, duration 00:01:56 + audio tracks: + 1, Unknown (AAC LC) (2.0 ch) (iso639-2: und) + subtitle tracks: Done. ```

Background on how this file came to be: I grabbed a few different interview clips that were originally separate titles on a DVD and used Blender to add a text scene with the question & the corresponding video so I could just have them all in one video file for my Plex server. First time I've used Blender to do this, so I didn't really pay any attention to the details of what it was spitting out. Just checked that it produced a video file that contained & played what I expected it to.

Not much of an interruption really. gsub looked to be a string manipulation function or something, so it seemed natural to look for parameters that dealt with text (like audio names, subtitles, etc). Just divide-and-conqure the parameters. Didn't really take too long to figure it out.

Only reason I thought that might be the place is it looks like that bit is grabbing the audio name from a collecting & calling gsub on it (which is what the displayed undefined method was anyways). Assuming Ruby lets you jam nil values into a collection, that just seemed like an obvious place. (If I knew anything about debugging Ruby (or had the time to figure it out) I'd probably just try to find it myself.)

lisamelton commented 5 years ago

@faithfracture Thanks for the quick response! And from the dump of the media.info hash in your scan output I can see :name=>nil there. Which is causing the following code in the prepare_audio function in the actual transcode-video source file problems:

        info = media.info[:audio][track]
        copy = (@copy_audio.first == :all or @copy_audio.include? track)

        if @copy_audio_name.first == :all or @copy_audio_name.include? track
          name = info.fetch(:name, '')
        else
          name = ''
        end

        name = @audio_name.fetch(track, name).gsub(/,/, '","')

That name = info.fetch(:name, '') line is the problem because it's stupidly assuming that :name won't be nil. D'oh!

Again, my apologies for this bug.

I should be a able to get a fix out sometime this week. Stay tuned and thanks again for reporting the issue!

faithfracture commented 5 years ago

Six stages of debugging

lisamelton commented 5 years ago

@faithfracture Perfect! 🤣

faithfracture commented 5 years ago

I should make a version of that that includes the amount of yelling & swearing at your computer that occurs at each stage, as well as the embarrassment & humility that comes with the last two stages, lol.

lisamelton commented 5 years ago

@faithfracture OK, the fix is in: https://github.com/donmelton/video_transcoding/commit/60ccb3eb283a138e0b517d91785e201d9ebdf28c

Should be released sometime later today.

faithfracture commented 5 years ago

Awesome! Thanks @donmelton!

lisamelton commented 5 years ago

@faithfracture OK, it's released Just gem update video_transcoding and you'll get the fix.

Thanks again for opening this issue!