Fixed the AudioDecoder class

[adlarkin]

I've updated the unit tests for the AudioDecoder class so that they compile and pass.

I also added a check in AudioDecoder::SampleRate to ensure that a null pointer is not de-referenced in case users call AudioDecoder::SampleRate before calling AudioDecoder::SetFile.

One question: In the case where users call AudioDecoder::SampleRate before calling AudioDecoder::SetFile, what should AudioDecoder::SampleRate return? I made AudioDecoder::SampleRate return -1 in this case, because I figured that a negative sample rate implies that the decoder's current state is invalid. I anyone thinks there is a better value to return (for example, 0), let me know, and I will update AudioDecoder::SampleRate accordingly.

[codecov]

Codecov Report

Merging #81 into ign-common3 will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@               Coverage Diff               @@
##           ign-common3      #81      +/-   ##
===============================================
+ Coverage        73.72%   73.94%   +0.21%     
===============================================
  Files               68       69       +1     
  Lines             9287     9390     +103     
===============================================
+ Hits              6847     6943      +96     
- Misses            2440     2447       +7     

Impacted Files	Coverage Δ
av/src/AudioDecoder.cc	85.43% <100.00%> (ø)
av/src/Util.cc	100.00% <0.00%> (+10.34%)	⬆️
av/src/ffmpeg_inc.cc	25.00% <0.00%> (+25.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6f7d2d...0038102. Read the comment docs.

[mjcarroll]

Rebase on ign-common3 and signoff for DCO please.

[scpeters]

One question: In the case where users call AudioDecoder::SampleRate before calling AudioDecoder::SetFile, what should AudioDecoder::SampleRate return? I made AudioDecoder::SampleRate return -1 in this case, because I figured that a negative sample rate implies that the decoder's current state is invalid. I anyone thinks there is a better value to return (for example, 0), let me know, and I will update AudioDecoder::SampleRate accordingly.

-1 sounds reasonable to me, and you documented it in the header file, so it looks fine to me

[iche033]

changes look good to me. Just need to rebase for abi-checker build to pass

[mjcarroll]

@scpeters and @iche033 I'm trying to help Ashton understand the output of the ABI report here:
https://build.osrfoundation.org/job/ignition_common-abichecker-any_to_any-ubuntu_auto-amd64/251/API_5fABI_20report/

Based on the changes in the PR, I wouldn't anticipate any API or ABI problems, because nothing along those lines has changed.

As far as I can tell the checker believes that essential the same symbol (mod template specialization) is being removed and added. What is interesting is that this cc file wasn't part of the build until this pull request.

If I look at my local copies of libignition-common3, that symbol doesn't exist:

$ nm -D /usr/lib/x86_64-linux-gnu/libignition-common3-av.so.3 | grep AudioDecoder
$ nm -D /usr/lib/x86_64-linux-gnu/libignition-common3.so.3 | grep AudioDecoder

[scpeters]

I'm trying to help Ashton understand the output of the ABI report here:
https://build.osrfoundation.org/job/ignition_common-abichecker-any_to_any-ubuntu_auto-amd64/251/API_5fABI_20report/

that looks like a false positive, which I've seen before: gazebo-tooling/release-tools#120

no code in that header file is changed, so I would just ignore it. an administrator should override the blocked merge in this case

[iche033]

oh I didn't know about the abi-checker bug. Not sure if it's an issue with the abi-checker when the base version contains only the header files and the .cc files were not being built.

[adlarkin]

Thanks for following up on this, @scpeters and @mjcarroll. Is everything okay then, or are there any other changes I need to make?