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

Fix when it is not buffer.hasRemaining(refs #144 #153) #158

Closed
wants to merge 1 commit into from

Conversation

jumperson
Copy link
Contributor

@jumperson jumperson commented Dec 23, 2021

There was a case where buffer.isremining() was false when used with AudioEngine.
When I debugged, the capacity of buffer was 0.

スクリーンショット 2021-12-22 20 49 19

refs: #144 #153

@jumperson jumperson changed the title Fix when it is not buffer.hasRemaining(Fix #153 #144) Fix when it is not buffer.hasRemaining(refs #144 #153) Dec 23, 2021
@jumperson
Copy link
Contributor Author

@natario1
Thank you for developing a great library.
I would appreciate it if you could review the fix.

@Tom3652
Copy link

Tom3652 commented Feb 2, 2022

I also need this fix please and thank you @jumperson.
However, will putting an if statement have an impact on the audio or not?

@natario1
Copy link
Owner

natario1 commented Mar 12, 2022

Thanks! I would prefer if we understand why buffer is empty. Where is it coming from? Seems like a bug upstream (either in Transcoder code, or in device decoders) that we should fix. Also with the proposed solution the buffer is never released.

@oyama202107
Copy link

when using com.otaliastudios:transcoder:0.10.4", I have same issue.

java.lang.IllegalArgumentException: bufferInfo must specify a valid buffer offset, size and presentation time
at android.media.MediaMuxer.writeSampleData(MediaMuxer.java:682)
at mb.b.d(DefaultDataSink.java:2)
at ib.d.d(eos.kt:6)
at eb.f.c(Writer.kt:11)
at gb.d.a(Pipeline.kt:1)
at ab.c.a(Segment.kt:7)
at hb.a.b(DefaultTranscodeEngine.kt:14)
at ya.a.call(Transcoder.java:25)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)

LiTr had same issue and it was fixed.
linkedin/LiTr#78
linkedin/LiTr#92

Last audio frame (with EOS flag) has a presentation time of -1, which causes an IllegalArgumentException on MediaMuxer when we try to write it. This was not happening on newer devices, because buffer size was zero, which prevents AudioTrackTranscoder from writing. But on some older devices (such as Samsung Galaxy S5) buffer size would be non-zero, which would make transcoding fail.

@jonandersen
Copy link

jonandersen commented Sep 6, 2022

@jumperson, @natario1 this fixes the issue for me, specifically any videos recorded on iOS seems to have this issue (using this fork https://github.com/StudistCorporation/Transcoder)

@vanniktech
Copy link
Contributor

@natario1 is there anything I can help with so that this would get merged and a new version would be published?

@natario1
Copy link
Owner

As I said before, ideally we should understand where this buffer is coming from, it seems like we are patching a bug that is somewhere upstream in the pipeline.

But if no one has time to do so, at least the buffer should be released in an else branch, otherwise it's leaked (I think). I can merge the PR after this, and could also use some help with publishing a new version.

@vanniktech
Copy link
Contributor

@natario1 there you go: #182

@natario1
Copy link
Owner

Looks like sonatype is down now https://status.maven.org/incidents/pplp3ln3vs81 - when they fix it you'll be able to use the snapshot version.

For a real release I would appreciate if someone made a simple version bump PR like this, listing the fixes since previous version.

@vanniktech
Copy link
Contributor

There you go @natario1: #183

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

Successfully merging this pull request may close these issues.

None yet

6 participants