Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--bind-srt-encoding or --bind-srt-language flags cause recursive array join error #37

Closed
bpharriss opened this issue Jan 9, 2016 · 16 comments

Comments

@bpharriss
Copy link

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.

@lisamelton
Copy link
Owner

@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...

@lisamelton
Copy link
Owner

@bpharriss BTW, can you give me some more information like what OS platform and which version of HandBrake you're using? Thanks.

@bpharriss
Copy link
Author

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.

@lisamelton
Copy link
Owner

@bpharriss OK, this may be caused by a patch I took from @arikalish then:

#25

But my code before his patch was clearly wrong. So it's possible the change just exposed some other bug.

@arikalish
Copy link
Contributor

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.

@lisamelton
Copy link
Owner

@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.

@arikalish
Copy link
Contributor

They're everyone's bugs now :)

@lisamelton
Copy link
Owner

@arikalish I'm just generous that way. :)

@lisamelton
Copy link
Owner

@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...

@arikalish
Copy link
Contributor

@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.

@lisamelton
Copy link
Owner

@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.

@arikalish
Copy link
Contributor

Damn... my first accepted pull request and it's got a bug... embarrassing. :)

@lisamelton
Copy link
Owner

@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.

@bpharriss
Copy link
Author

Good catch. And get back to the important stuff - that wine isn't going to sip itself ;)

@lisamelton
Copy link
Owner

@bpharriss Damn right! :)

@lisamelton
Copy link
Owner

@bpharriss and @arikalish OK, the fix is in and pushed. Just gem update to get version 0.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants