Cracktro :)

The Partridge Family were neither partridges nor a family. Discuss.
albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 9:03 pm

The "safe"pointer function is an absolute disgusting hack, no doubt about it. I have to redo that properly. It will be less efficient afterwards though. You see, I have to pass the sample data to the constructor of the Sample class that does copy the data. If I copy it in the getsafepointer fn, then the sample data gets copied twice over... Or more accurately, it gets copied, then converted (all unsigned data is converted to signed for the internal mixer, all 8 bit data is converted to 16 bit data).
I was wondering about that function and the way you use it in my opinion is valid. You are getting a pointer to some bytes from a buffer, though I personally think returning a char* instead of void* is more correct.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » November 9th, 2019, 11:56 am

I suppose I should use a shared_pointer in this case. I might refactor it later, I want to give more priority to the loaders. I also need to break up mod_to_wav.cpp into one or more header files and just a simple cpp file with a main() function for testing.

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 9th, 2019, 5:37 pm

What you have in the VirtualFile is a unique_ptr and some raw pointers showing current read/write position and end of file/stream. Returning a raw char*, char const* or or char const* const would be appropriate depending on how the return value is used.

char* as a return when you want to directly write to the buffer/stream
char const* when you only want to allow reading
char const* const when the returned pointer shouldn't be changed and is read only
char *const when the returned pointer shouldn't be changed and is read/write

Code: Select all

int16_t const* const constSampleConstPtr = reinterpret_cast<int16_t const* const>( file.getSafePointer( offset ) );
++sample; // fail: pointer is constant
(*sample) = 42;  // fail: data is constant

int16_t * const constSampleConstPtr = reinterpret_cast<int16_t * const>( file.getSafePointer( offset ) );
++sample; // fail: pointer is constant
(*sample) = 42; // okay: data is not constant
Or instead of returning a void, another way is to use a template:

Code: Select all

template<typename Pointer>
Pointer getSafePointer( int index ){
    static_assert( std::is_pointer_v<Pointer>, "Pointer template variable must be a pointer type" );
    if( /* valid condition */ )
        return reinterpret_cast<Pointer>( filePos );
    else
        return nullptr;
}

// Call site
int16_t const* sample = file.getSafePointer<int16_t const*>( offset );
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » November 11th, 2019, 10:01 pm

Oh my, more studying material! Or rather, reference material. I believe I can make the pointer in VirtualFile more const indeed, have to look into it. I was trying to clean the XMPlayer some more, but I didn't make as much progress as I hoped. Also, started working on supporting Envelopes (volume, panning etc), something I have been avoiding for a while... For no obvious reason.
Thanks again for the help.

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 12th, 2019, 12:33 am

You're welcome. With that last post, I just wanted to express that the void* return type was the only issue I had, the body of the function seemed clear enough about what it does...I suppose the name doesn't really describe what it does, but I don't have any suggestions. :(

I thought about overloading the subscript operator ( operator[] ). However, that really doesn't fit with the design of the class. Simple read/write functions would accomplish the same, and be more familiar with the type of class VirtualFile mimics. Using the subscript operator might allow you to index a location as well as move the filePos to the new indexed location, but it can't be templated ( as far as I know ) and wouldn't offer the safety your getSafePointer() does.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Post Reply