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

Poor documentation and missing, mandatory features #63

Closed
codesoap opened this issue Jan 2, 2025 · 1 comment
Closed

Poor documentation and missing, mandatory features #63

codesoap opened this issue Jan 2, 2025 · 1 comment

Comments

@codesoap
Copy link

codesoap commented Jan 2, 2025

Summary

I'm interested in this library to decode Opus files, but the documentation is too sparse for me to feel confident choosing this library. It also seems to me, that it is missing features, that are required by RFC 6716, and there is no explanation for it. If the documentation were more clear and your design decisions more comprehensible, I would feel more comfortable choosing this library for my project.

Additional context

I'll provide some examples for what I miss in the documentation:

  1. The github.com/pion/opus package contains an exported Configuration type. This seems to be unused in the other exported types and functions, so it's purpose is unclear to me.
  2. The Decode method takes an output buffer via the out parameter. It is unclear to me, how large it should be and what effects its size have. The examples simply use a length of 1920, without any explanation.
  3. It is unclear to me, how I should treat the output of Decode differently, if isStereo == true. How is the second channel encoded in the PCM data?
  4. The Decode method outputs signed 16 bit samples, but the data type of out is an unsigned 8 bit integer. I assume, that it is the responsibility of the caller, to transform the 8 bit integers into 16 bit samples. What is the benefit of this complication? Why does the library not provide signed 16 bit samples directly?
  5. In section 2.1.2 of RFC 6716, the specification talks about decoding mono frames in stereo decoders and stereo frames in mono decoders. This library does not seem to support this kind of "transcoding". What was the reason for leaving these features out?
  6. In section 2.1.3 of RFC 6716, the specification requires, that every decoder must be able to decode at a different bandwidth, than the source is providing. This library does not seem to support this. Why did you choose to violate the specification here?

These are just the first few questions, that came into my mind when trying to understand this library. I'm sure many more would follow, if I were to use this library. It seems to me, like this library is very strongly opinionated in its design and even goes as far as violating the RFC. I think this should be more clearly communicated.

@JoeTurki
Copy link
Member

Hello!

Please refer to the roadmap or other issue. If there's a specific part of the spec or issue you'd like to implement, could you create separate issues? This will help us better track progress.

Also, this library could use some more love, if you can help with contribution, it would be greatly appreciated :)

Thank you

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

2 participants