Creating libao class objects. Why does audio only play once.

[cmisip]

Can't figure out why audio_sample objects only plays audio one time. Any ideas?

Thanks,
Chris

/*
 * To change this license header, choose License Headers in Project Properties.
 * To change this template file, choose Tools | Templates
 * and open the template in the editor.
 */

/* 
 * File:   main.cpp
 * Author: cmisip
 *
 * Created on March 4, 2017, 2:41 PM
 */


#include <ao/ao.h>
#include <mpg123.h>
#include <string>
#include <iostream>

#include <fcntl.h>
#include <cstring>
#include <unistd.h>

#define BITS 8

ao_device *dev;
int driver;

class audio_sample {
    
public:
    audio_sample(const char*  filename){
        mh = mpg123_new(NULL, &err);
        buffer_size = mpg123_outblock(mh);
        buffer = (unsigned char*) malloc(buffer_size * sizeof(unsigned char));

        int fnum = open(filename, O_RDONLY);
        if(fnum < 0){
            printf("ERROR opening file: %s\n", strerror(fnum));
            exit(0);
        }
    
        mpg123_open(mh, filename);
        mpg123_getformat(mh, &rate, &channels, &encoding);
        
        /* set the output format and open the output device */
        format.bits = mpg123_encsize(encoding) * BITS;
        format.rate = rate;
        format.channels = channels;
        format.byte_format = AO_FMT_NATIVE;
        format.matrix = 0; 
    };
    
    ~audio_sample(){
        free(buffer);
        mpg123_close(mh);
        mpg123_delete(mh);
    }
    
    void play(){
        dev = ao_open_live(ao_default_driver_id(), &format, NULL);
        /* decode and play */
        while (mpg123_read(mh, buffer, buffer_size, &done) == MPG123_OK)
           ao_play(dev, reinterpret_cast<char*>(buffer), done);
    }
    
    mpg123_handle *mh;
    unsigned char *buffer;
    size_t buffer_size;
    size_t done;
    int err;
    
    ao_sample_format format;
    int channels, encoding;
    long rate;
};

int main(int argc, char *argv[])
{
    
    /* initializations */
    ao_initialize();
    driver = ao_default_driver_id();
    
    int err = mpg123_init();
    if(err != MPG123_OK) {
       fprintf(stderr, "Basic setup goes wrong: %s", mpg123_plain_strerror(err));
    }
    
    //audio samples need to be a certain minimum length
    audio_sample dining_room("samples/Dining_Room.mp3"); 
    audio_sample living_room("samples/Living_Room.mp3");
    audio_sample kitchen("samples/Kitchen_Nook.mp3");
    audio_sample front_hallway("samples/Front_Hallway.mp3");
    
    dining_room.play();
    usleep(1000000);
    living_room.play();
    usleep(1000000);
    kitchen.play();
    usleep(1000000);
    front_hallway.play();
    
    //Repeat playback.  No audio plays from this point. 
    dining_room.play();
    usleep(1000000);
    living_room.play();
    usleep(1000000);
    kitchen.play();
    usleep(1000000);
    front_hallway.play();
    
    
    /* clean up */
    ao_close(dev);
    
    mpg123_exit();
    ao_shutdown();

    return 0;
}

[mbozzi]

I don't know for sure, but you are repeatedly opening the same device multiple times without closing it, and not error-checking.

https://www.xiph.org/ao/doc/ao_open_live.html
Specifies that in the case of failure, the return value is NULL and errno will be set. Try checking it.

[cmisip]

So basically, I need to move the :

dev = ao_open_live(ao_default_driver_id(), &format, NULL);

away from the class and into main and just pass a reference (or pointer) to it to the constructor of the libao class. The libao class must contain an ao_device pointer that the constructor can assign to when the object is created. I'm away from my cpp computer at this time but will try that idea.

Incidentally, I found out the reason for lack of playback on the second invocation of object.play. Apparently, the mpg123 handle keeps track of seek position. After first playback, the seek position goes to the end of the buffer. Resetting the seek position to the beginning of the buffer allows playback again. I have only done very limited testing though. Will work on the modifications and will probably have more questions about this topic.

Thanks,
Chris

[cmisip]

So this is the current working solution. I dont even need to use a usleep anymore after playback as the call to object.play seems to be blocking. I thought that was why audio was cutting out.

Questions:
1. Why is dev visible inside the class. I found this out by accident by using

ao_play(dev, reinterpret_cast<char*>(buffer), done);

inside the class ( I forgot to change it at first ).

2. It would seem as though this would be one of those situations where a move assignment operator would work but I can't seem to get that part of the code working. Any ideas?

3. Is there any mechanism that I might employ to ensure that when an audio_sample object is created, the ao initialization is completed first? If I comment out ao_initialize(), I will get a segfault. ao_initialize() is a prerequisite of creating valid audio_sample objects.

4. I am creating an unusable audio_sample object using the default constructor. Is this ok or bad practice? In one of the earlier versions of my project, it was necessary in order to have global scope for audio_sample objects created inside curly brackets.


Thanks,
Chris

/* 
 * File:   main.cpp
 * Author: cmisip
 *
 * Created on March 4, 2017, 2:41 PM
 */


#include <ao/ao.h>
#include <mpg123.h>
#include <string>
#include <iostream>

#include <fcntl.h>
#include <cstring>
#include <unistd.h>

#define BITS 8

ao_device *dev;
int driver;
ao_sample_format format;




class audio_sample {
    
public:
    audio_sample(){};
    audio_sample(const char*  ifile, ao_device * a_idev):a_dev(a_idev){
        init();
        filename=ifile;
        process();
    };
    
    void init(){
        mh = mpg123_new(NULL, &err);
        buffer_size = mpg123_outblock(mh);
        buffer = (unsigned char*) malloc(buffer_size * sizeof(unsigned char));
    
    }
    
    void process() {
        int fnum = open(filename, O_RDONLY);
        if(fnum < 0){
            printf("ERROR opening file: %s\n", strerror(fnum));
            exit(0);
        }
    
        int retval=mpg123_open(mh, filename);
        close(fnum);
    }
    
    audio_sample & operator=(const audio_sample &other) {
        std::cout << "Assignment operator "<< std::endl;
        if(this != &other ){
           init();
           filename=other.filename;
           a_dev=other.a_dev;
           process(); 
        }
        
        return *this;
    }
    
    ~audio_sample(){
        free(buffer);
        mpg123_close(mh);
        mpg123_delete(mh);
    }
    
    void play(){
        
        /* decode and play */
        while (mpg123_read(mh, buffer, buffer_size, &done) == MPG123_OK)
           ao_play(a_dev, reinterpret_cast<char*>(buffer), done);
        mpg123_seek(mh,0,SEEK_SET);
    }
    
    mpg123_handle *mh;
    unsigned char *buffer;
    size_t buffer_size;
    size_t done;
    int err;
    
    
    int channels, encoding;
    long rate;
    ao_device *a_dev;
    const char * filename;
};


audio_sample dining_room, living_room, kitchen, front_hallway;

int main(int argc, char *argv[])
{
    
    /* initializations */
    ao_initialize();
    driver = ao_default_driver_id();
    format.bits = 16;
    format.rate = 44100;
    format.channels = 2;
    format.byte_format = AO_FMT_NATIVE;
    format.matrix = 0; 
    
    dev = ao_open_live(ao_default_driver_id(), &format, NULL);
    
    int err = mpg123_init();
    if(err != MPG123_OK) {
       fprintf(stderr, "Basic setup goes wrong: %s", mpg123_plain_strerror(err));
    }
    
    if (true) {
      dining_room=audio_sample("samples/Dining_Room.mp3",dev); 
      living_room=audio_sample("samples/Living_Room.mp3", dev);
      kitchen=audio_sample("samples/Kitchen_Nook.mp3",dev );
      front_hallway=audio_sample("samples/Front_Hallway.mp3",dev);
    }
    
    
    dining_room.play();
    living_room.play();
    kitchen.play();
    front_hallway.play();
    
    //Repeat
    dining_room.play();
    living_room.play();
    kitchen.play();
    front_hallway.play();
    
    /* clean up */
    ao_close(dev);
    
    mpg123_exit();
    ao_shutdown();

    return 0;
}

[mbozzi]

1. Why is dev visible inside the class.

Because line 20 defines it in the global namespace.

2. It would seem as though this would be one of those situations where a move assignment operator would work but I can't seem to get that part of the code working. Any ideas?

You should not implement the rule of five for this class, and instead rely on the standard library utilities to directly handle your resources. Supply a custom deleter for std::unique_ptr, and let it represent ownership of the resources.

Also, what's going on with that copy-assignment operator? You're leaking memory.
A more correct way to write the copy-assignment operator involves reusing *this and copy-constructing the new object in place:

audio_sample& operator=(audio_sample const& lhs) {
  if (this != std::addressof(lhs)) {
    this->~audio_sample(); // Lifetime of *this ends; all resources freed.
    new (this) audio_sample (lhs); // Storage reuse is well-defined.
  }
  return *this;  
}

But this isn't the end of the story. In the case where copy construction may fail, you're out of luck because all of *this's data was just destroyed. Look into the copy-and-swap idiom.

3. Is there any mechanism that I might employ to ensure that when an audio_sample object is created, the ao initialization is completed first? If I comment out ao_initialize(), I will get a segfault. ao_initialize() is a prerequisite of creating valid audio_sample objects.

One simple approach is to create a function maybe_ao_init() which does the requisite initialization if it hasn't happened yet (use some global state to keep track of it).

It's up to you whether that's a good idea. Keep in mind that the state of the library is ultimately up to the library, not you: will your information ever be wrong? What happens if it's out of sync?

[cmisip]

For some reason I thought global variables are not visible in class definitions except if they are passed through the constructor. But its probably best to send dev through the constructor. It will be a good check for valid ao initialization (which I will put in later when I figure it out).

I also remembered that I forgot the std=c++11 parameter to the compiler so it was generating errors with move assignment. I just wanted to explore that avenue to see if it is possible and if it truly does provide some optimization.

Had to create non-useable audio_sample objects in global scope (program design) with default constructor which does not really do anything. So a check for NULL pointer value would let me know if memory was actually allocated that needed to be deallocated prior to copy assignment or move assignment, until I found out that such check is not necessary since deallocating null pointers is safe. The only step necessary was to make sure the pointer members of the class are initialized to NULL.

This is helping me along in my understanding of move semantics. I hope I got things right.

The following code is still confusing.

new (this) audio_sample (other);

It calls the copy constructor and the created object is put in the memory allocated for this. It seems technically a move assignment but it involved making a copy.

Do you see any memory leaks?

Thanks,
Chris

/* 
 * File:   main.cpp
 * Author: cmisip
 *
 * Created on March 4, 2017, 2:41 PM
 */


#include <ao/ao.h>
#include <mpg123.h>
#include <string>
#include <iostream>

#include <fcntl.h>
#include <cstring>
#include <unistd.h>

#define BITS 8





class audio_sample {
    
public:
    audio_sample(){ std::cout << "Default Constructor" << std::endl;};
    
    audio_sample(const char*  ifile, ao_device * a_idev):a_dev(a_idev){
        std::cout << "Parameterized Constructor" << std::endl;
        init();
        filename=ifile;
        process();
    };
    
    void init(){
        mh = mpg123_new(NULL, &err);
        buffer_size = mpg123_outblock(mh);
        buffer = (unsigned char*) malloc(buffer_size * sizeof(unsigned char));
    
    }
    
    void process() {
        int fnum = open(filename, O_RDONLY);
        if(fnum < 0){
            printf("ERROR opening file: %s\n", strerror(fnum));
            exit(0);
        }
    
        int retval=mpg123_open(mh, filename);
        close(fnum);
        mpg123_read(mh, buffer, buffer_size, &done);
    }
    
    
    
    audio_sample(const audio_sample &other){  //LESSON: must be const parameter
        std::cout << "Copy constructor ..calls copy assignment"<< std::endl;
        *this=other;  
    }
    
    
    audio_sample & operator=(const audio_sample &other) { //LESSON: must be const parameter
            std::cout << "Copy Assignment operator "<< std::endl;
            if(this != &other ){
               this->~audio_sample(); //LESSON: OK to delete a null pointer
            
               
            init();
            buffer_size=other.buffer_size;
            filename=other.filename;
            a_dev=other.a_dev;
            process();
            
            
        }
        return *this;
    }
    
    audio_sample( audio_sample &&other){ //LESSON: cannot be const parameter
        std::cout << "Move constructor ..calls move assignment"<< std::endl;
           *this=std::move(other);
          
    }
    
   
     audio_sample & operator=(audio_sample &&other) { //LESSON: cannot be const parameter
            std::cout << "Move Assignment operator "<< std::endl;
            if(this != &other ){
               this->~audio_sample(); //LESSON: OK to delete a null pointer
            
            //METHOD 1 : std::move --> WORKS
            /*mh=std::move(other.mh);
            other.mh=nullptr;   //LESSON: std::move does not set moved from pointer to nullptr
            buffer=std::move(other.buffer);
            other.buffer=nullptr;
            buffer_size=std::move(other.buffer_size);
            filename=std::move(other.filename);
            a_dev=std::move(other.a_dev);
            */
            
            //METHOD 2: copy values -->WORKS
            /*mh=other.mh;
            other.mh=nullptr;
            buffer_size=other.buffer_size;
            buffer=other.buffer;
            other.buffer=nullptr;
            filename=std::move(other.filename);
            a_dev=std::move(other.a_dev);
            */
             
           //METHOD 3: in place construction --> WORKS
            new (this) audio_sample (other);   //calls copy constructor which calls copy assignment
                                               //puts the created object in memory allocated for this
                                               //Technically, this seems like move assignment
            
            
        }
        return *this;
        
    }
  
    
    ~audio_sample(){
        std::cout << "Destructor" << std::endl;
        free(buffer);
        mpg123_close(mh);
        mpg123_delete(mh);
    }
    
    void play(){
        mpg123_seek(mh,0,SEEK_SET);
        /* decode and play */
        while (mpg123_read(mh, buffer, buffer_size, &done) == MPG123_OK)
           ao_play(a_dev, reinterpret_cast<char*>(buffer), done);
        
    }
    
    mpg123_handle *mh=NULL;   //LESSON: cpp does not initialize pointers to NULL by default
    unsigned char *buffer=NULL;
    size_t buffer_size;
    size_t done;
    int err;
    
    
   
    ao_device *a_dev;
    const char * filename;
};




int main(int argc, char *argv[])
{
    
    /* initializations */
    ao_initialize();
    ao_sample_format format;
    int driver;
    
    driver = ao_default_driver_id();
    format.bits = 16;
    format.rate = 44100;
    format.channels = 2;
    format.byte_format = AO_FMT_NATIVE;
    format.matrix = 0; 
    
    ao_device *dev;
    dev = ao_open_live(ao_default_driver_id(), &format, NULL);
    
    int err = mpg123_init();
    if(err != MPG123_OK) {
       fprintf(stderr, "Basic setup goes wrong: %s", mpg123_plain_strerror(err));
    }
    
    std::cout << "Default constrution---->DEFAULT_CONSTRUCTOR " << std::endl;
    audio_sample  back_hallway;  //default constructed objects that are not useable
    
    
    std::cout << "Parameterized construction--->PARAM_CONSTRUCTOR" << std::endl;
    audio_sample b_hallway("samples/Front_Hallway.mp3",dev);  //parameterized construction, useable object
    b_hallway.play();
    audio_sample d_room("samples/Dining_Room.mp3",dev);
    d_room.play();
    audio_sample l_room("samples/Living_Room.mp3",dev);
    l_room.play();
    
    std::cout << "Useable object with useable object to be retained-->COPY ASSIGNMENT" << std::endl;
    b_hallway=d_room;  
    b_hallway.play();  //plays
            
    std::cout << "Useable object with useable object to be expired-->MOVE ASSIGNMENT" << std::endl;
    b_hallway=std::move(d_room);  //dont use d_room anymore 
    b_hallway.play();  //plays
    
    std::cout << "Useable object with temporary object-->MOVE ASSIGNMENT" << std::endl;
    b_hallway=audio_sample("samples/Front_Hallway.mp3",dev);  //assigning a useable object with a useable temporary
    b_hallway.play();  //plays
    
    std::cout << "Nonuseable object with temporary object--->MOVE ASSIGNMENT" << std::endl;
    back_hallway=audio_sample("samples/Front_Hallway.mp3",dev);  //assigning a nonuseable object with a useable temporary
    back_hallway.play();  //plays
    
    std::cout << "Create object with existing object--->COPY CONSTRUCTOR" << std::endl;
    audio_sample dining_room(l_room);  
    dining_room.play();  //plays
    
    std::cout << "Create object with existing object to be expired--->MOVE CONSTRUCTOR" << std::endl;
    audio_sample f_hall(std::move(l_room));  
    f_hall.play();  //plays
    
    /* clean up */
    ao_close(dev);
    
    mpg123_exit();
    ao_shutdown();

    return 0;
}