Implement common SinkError and SinkResult

[JasonLG1979]

• Implement common SinkError and SinkResult for use with all backends.

• impl From<AlsaError> for SinkError so AlsaErrors can be thrown to player as SinkErrors.

• impl From<PulseError> for PulseError so PulseErrors can be thrown to player as SinkErrors.

• Other backends can follow the same pattern as the alsa and pulseaudio backends for detailed errors in the future if someone wishes to implement them.

• A little code clean up and added blank lines to make it easier to read at a glance.

[roderickvd]

While I like this direction, I would prefer to not throw as io::Error but rather a SinkError. For inspiration take a look at how the decoder currently handles this.

[JasonLG1979]

While I like this direction, I would prefer to not throw as io::Error but rather a SinkError. For inspiration take a look at how the decoder currently handles this.

You're talking about changing the return type in mod. That would instantly break every backend. That would mean I would have to fix all of the backends at once basically. That's not really something I want to do,lol!!!

I kinda like the idea of keeping io::Error as an interim stop gap. That allows us (hopefully other people will help?) to fix the backends one at a time and then finally when they're all up to snuff then we can implement SinkError.

From a possible regression/error standpoint I also think it would be better to make each backend fix a separate PR. It also makes for smaller jobs and easier to review code.

[JasonLG1979]

When all of the backends are eventually fixed by hopefully other people who actually use and care about them I volunteer to implement SinkError. I really only care that much about the PulseAudio and ALSA backends since that's basically I'm guessing what 99% of Linux users actually use. Pipe and subprocess I have to care about because they comes along for the ride no matter what but the other backends I care and know basically nothing about.

[roderickvd]

I'll think about it. I don't think changing it in all the backends would be that much work (as long as you keep further error improvements separate).

[JasonLG1979]

I'll think about it. I don't think changing it in all the backends would be that much work (as long as you keep further error improvements separate).

There. You were right, it wasn't that bad. For the most part since most of the backends just unwrap and explode instead of actually throwing errors it was mostly just a matter of changing the return type on functions they override.

I'm not married to the error names or strings. I was just trying to keep it simple. I'm open to any and all suggestions.

[JasonLG1979]

@roderickvd on an unrelated note I put in a PR to the alsa bindings so hopefully we can stop worrying about endianness in our ALSA backend. Not that it's a huge concern but still.

[roderickvd]

I see they were already merged upstream, great!

[JasonLG1979]

I see they were already merged upstream, great!

Yep so I guess when the next version is released we can use Format::s24_3().

[JasonLG1979]

@roderickvd I think this about ready for a review when you get a chance.

[roderickvd]

Shall we do #823 first?

[JasonLG1979]

Shall we do #823 first?

Sure. You've already started the review process. No matter the order I will need do a little bit of tweaking on the one that gets merged 2nd anyway. I tried to make them contained as possible but it was inevitable that there would be a little bit of bleed though.

[roderickvd]

Alright, last one!

[JasonLG1979]

There's a little bit of work to be done in resolving the conflicts beyond what github would have you believe. I'll give it a go after work over the next day or 2.

[JasonLG1979]

If all goes well that should do it as far as fixing the conflicts.

[JasonLG1979]

Ok @roderickvd I think this is done for real this time 😃 That last one was just a bonus. 😆

[roderickvd]

Great! I'll check it out and I think we have a 0.3.0 on our hands!

[roderickvd]

Working fine for me so far ❤️ let's merge this, give it a bit of exposure and ping @sashahilton00 for a release somewhere the coming weekends?