--bind-srt-encoding or --bind-srt-language flags cause recursive array join error
bpharriss opened this issue · 16 comments
ran transcode-video with the following arguments:
transcode-video --add-srt /Volumes/BitBucket/batch-mkv/Subtitles/Paper\ Towns.srt --bind-srt-language eng --bind-srt-encoding UTF-8 --mp4 /Volumes/BitBucket/MakeMKV\ Destination/Paper\ Towns.mkv
I get an error returned:
/usr/bin/transcode-video: recursive array join
If I remove either --bind-srt-language OR --bind-srt-encoding but leave the other flag in, I still get the error. Removing both flags causes the error to not occur and the transcoding completes successfully.
@bpharriss OK, I'll see if I can reproduce with my test files after dinner. I'll also inspect the code to see how that particular error message might happen.
Stay tuned...
@bpharriss BTW, can you give me some more information like what OS platform and which version of HandBrake you're using? Thanks.
HandBrake is 0.10.2. I'm running on a 10.9.5 system. All was working correctly, then I decided to "brew upgrade" and "gem update" and then the issue started.
@bpharriss OK, this may be caused by a patch I took from @arikalish then:
But my code before his patch was clearly wrong. So it's possible the change just exposed some other bug.
In all truth, I've stopped pulling in .srts manually lately and instead started remuxing the source mkvs with the srt and DVD-style subs converted from the PGS subs (converting my Bluray library) so I haven't used the command line srt options and wouldn't have come across this since then.
It's definitely possible that my patch borked something but it was super-simple.
@arikalish Yeah, super simple. You can see from the diff that what I was doing before was clearly stupid. Probably some cut and paste error. :) My theory is that your fix simply exposed another one of my bugs.
They're everyone's bugs now :)
@arikalish I'm just generous that way. :)
@bpharriss @arikalish Sonuvagun! Rolling back the change "fixes" the problem. However, I suspect this is because the offset code never worked correctly. Ugh.
Still tinkering with it...
@bpharriss an easy way to check if it ever worked might be to give v.27 a bogus encoding (say, EBDAC or maybe something a little less esoteric...) and see if it totally munges the output.
@arikalish I found the bug. There was a typo in your patch. And we both missed it. :)
It should have changed "language" to "offset," not "offsets." So... not plural. :)
I'll fix it shortly.
Damn... my first accepted pull request and it's got a bug... embarrassing. :)
@arikalish Hey, I missed it too. And I screwed up the regression test. :)
@bpharriss I should have an updated Gem out later tonight after some more testing (and after dinner). My apologies.
Good catch. And get back to the important stuff - that wine isn't going to sip itself ;)
@bpharriss Damn right! :)
@bpharriss and @arikalish OK, the fix is in and pushed. Just gem update to get version 0.3.1.